Fixed build fail when NO_PROTOCHAIN specified

[ppaul1989]

Build on latest master:

Line gencode.c:6043

static struct block * gen_protochain(compiler_state_t *cstate, bpf_u_int32 v, int proto) { 
#ifdef NO_PROTOCHAIN 
   -> return gen_proto(cstate, v, proto); 
#else

Error

Scanning dependencies of target SerializeTarget
[  2%] Generating grammar.c, grammar.h
[  5%] Generating scanner.c, scanner.h
/home/pbrzezinski/open-source-contrib/libpcap/grammar.y:40.1-7: warning: POSIX Yacc does not support %expect [-Wyacc]
 %expect 38
 ^~~~~~~
[  5%] Built target SerializeTarget
Scanning dependencies of target pcap_static
[  8%] Building C object CMakeFiles/pcap_static.dir/bpf_filter.c.o
[ 11%] Building C object CMakeFiles/pcap_static.dir/bpf_dump.c.o
[ 14%] Building C object CMakeFiles/pcap_static.dir/gencode.c.o
[ 17%] Building C object CMakeFiles/pcap_static.dir/bpf_image.c.o
[ 20%] Building C object CMakeFiles/pcap_static.dir/etherent.c.o
[ 22%] Building C object CMakeFiles/pcap_static.dir/fmtutils.c.o
[ 25%] Building C object CMakeFiles/pcap_static.dir/optimize.c.o
/home/pbrzezinski/open-source-contrib/libpcap/gencode.c: In function ‘gen_protochain’:
/home/pbrzezinski/open-source-contrib/libpcap/gencode.c:6043:9: error: too few arguments to function ‘gen_proto’
  return gen_proto(cstate, v, proto);
         ^~~~~~~~~
/home/pbrzezinski/open-source-contrib/libpcap/gencode.c:571:22: note: declared here
 static struct block *gen_proto(compiler_state_t *, bpf_u_int32, int, int);
                      ^~~~~~~~~
[ 28%] Building C object CMakeFiles/pcap_static.dir/nametoaddr.c.o
make[2]: *** [CMakeFiles/pcap_static.dir/build.make:142: CMakeFiles/pcap_static.dir/gencode.c.o] Error 1
make[2]: *** Waiting for unfinished jobs....
[ 31%] Building C object CMakeFiles/pcap_static.dir/pcap.c.o
[ 34%] Building C object CMakeFiles/pcap_static.dir/pcap-common.c.o
make[1]: *** [CMakeFiles/Makefile2:73: CMakeFiles/pcap_static.dir/all] Error 2
make: *** [Makefile:130: all] Error 2

How I build :

cd build
cmake cmake -DBUILD_SHARED_LIBS=OFF -DNO_PROTOCHAIN=ON ..
cmake --build .

[ppaul1989]

Apart from build problem dir seems not used in any meaningful way anymore here.

[mcr]

So I think that we included the direction in order to provide future proofing against BPF code that would be able to distinguish between incoming and outgoing packets.

[infrastation]

Another way to reproduce the problem is using ./configure --disable-protochain. As Michael notes above, the [now effectively unused] dir argument to gen_proto() seems to serve as a loose guard for future code, so let's leave it as it is for now.

This way, at a glance the following change would be a much simpler and more consistent way to fix the error:

--- a/gencode.c
+++ b/gencode.c
@@ -6083,7 +6083,7 @@ static struct block *
 gen_protochain(compiler_state_t *cstate, bpf_u_int32 v, int proto)
 {
 #ifdef NO_PROTOCHAIN
-       return gen_proto(cstate, v, proto);
+       return gen_proto(cstate, v, proto, Q_DEFAULT);
 #else
        struct block *b0, *b;
        struct slist *s[100];

However, it means "instead of chasing the protocol chain just test that the outermost protocol number of the packet matches the protocol number given in the expression", which does not look right: what the filter expression looks should be what it actually does, so long as it manages to compile.

However, upon a closer look it is possible to notice that with NO_PROTOCHAIN defined it should not matter what gen_protochain() does because it should not be called because grammar.y.in rejects the keyword in the first place:

#ifdef NO_PROTOCHAIN
				  bpf_set_error(cstate, "protochain not supported");
				  YYABORT;
#else
				  QSET($$.q, $1, Q_DEFAULT, Q_PROTOCHAIN);
#endif

This actually works, which is good because the library rejects the filter immediately with an error and the user can tell the reason:

$ ./tcpdump -y EN10MB -d 'ip protochain \ah'
tcpdump: protochain not supported

However, this was not quite exactly the case because two calls to gen_protochain() were conditional on CHASE_CHAIN. Although the latter was never defined, it complicated the logic still.

After some time studying the matter I found it better to remove CHASE_CHAIN altogether in commit e6129d5. After that the solution to the NO_PROTOCHAIN problem became straightforward and was implemented in commit bc594f1. Thank you for reporting this problem and for suggesting a possible solution. Closing as resolved.

[guyharris]

It looks as if the intent of CHASE_CHAIN was to make all protocol tests do protocol chasing, e.g. even tcp, proto \tcp, and ip proto \tcp would do protocol chasing, which would means there's no way not to get protocol chasing.

If we could make protocol chasing work everywhere, including in the kernel, and support optimization of filters with backwards branches - i.e., the only cost of protocol chasing would be a little CPU time in the filter, not "this has to be done in userland" or "filter code isn't optimized"" - that might be a nice simplification (although the place where protocol chasing seems to be most important is with VLAN traffic, but we don't support any way to chase through VLAN headers).

But, as we don't, I don't see much point in keeping the CHASE_CHAIN stuff around.