On Sat, Nov 10, 2018 at 02:01:56PM +0100, Klemens Nanni wrote:
> On Sat, Nov 10, 2018 at 10:05:18AM +0100, Denis Fondras wrote:
> > 'tcpdump vlan $vlan_number' will not return anything if priority is not 0.
> >
> > Fix that by not comparing vlan number against priority.
> > Also deny usage of vlan number greater than 4095 while at it.
> 4095 is reserved, 4094 is the maximum.
>
I do not agree. 4095 is reserved and has special meaning in some/most
implementation but is not prohibited nor invalid.
Anyone with a strong opinion about it ?
> OK kn with using defines, see inline with one other irrelevant nit.
>
> > @@ -3364,6 +3364,11 @@ gen_vlan(vlan_num)
> > /*NOTREACHED*/
> > }
> >
> > + if (vlan_num > 4095) {
> /usr/include/netinet/if_ether.h
> 99:#define EVL_VLID_MAX 0xFFE
>
> > + bpf_error("invalid VLAN number : %d", vlan_num);
> > + /*NOTREACHED*/
> Adding more of these obsolete hints seems off. I see how it keeps
> consistency, but it's not a big deal and we might even just sweep the
> existing ones.
>
I am fine with that.
> > + }
> > +
> > /*
> > * Change the offsets to point to the type and data fields within
> > * the VLAN packet. This is somewhat of a kludge.
> > @@ -3395,7 +3400,7 @@ gen_vlan(vlan_num)
> > if (vlan_num >= 0) {
> > struct block *b1;
> >
> > - b1 = gen_cmp(orig_nl, BPF_H, (bpf_int32)vlan_num);
> > + b1 = gen_mcmp(orig_nl, BPF_H, (bpf_int32)vlan_num, 0x0FFF);
> 95:#define EVL_VLID_MASK 0xFFF
Index: gencode.c
===================================================================
RCS file: /cvs/src/lib/libpcap/gencode.c,v
retrieving revision 1.51
diff -u -p -r1.51 gencode.c
--- gencode.c 10 Nov 2018 10:17:37 -0000 1.51
+++ gencode.c 11 Nov 2018 08:52:27 -0000
@@ -3359,15 +3359,11 @@ gen_vlan(vlan_num)
{
struct block *b0;
- if (variable_nl) {
+ if (variable_nl)
bpf_error("'vlan' not supported for variable DLTs");
- /*NOTREACHED*/
- }
- if (vlan_num > 4095) {
+ if (vlan_num > 4095)
bpf_error("invalid VLAN number : %d", vlan_num);
- /*NOTREACHED*/
- }
/*
* Change the offsets to point to the type and data fields within
@@ -3389,7 +3385,6 @@ gen_vlan(vlan_num)
default:
bpf_error("no VLAN support for data link type %d",
linktype);
- /*NOTREACHED*/
}
}
@@ -3400,7 +3395,8 @@ gen_vlan(vlan_num)
if (vlan_num >= 0) {
struct block *b1;
- b1 = gen_mcmp(orig_nl, BPF_H, (bpf_int32)vlan_num, 0x0FFF);
+ b1 = gen_mcmp(orig_nl, BPF_H, (bpf_int32)vlan_num,
+ EVL_VLID_MASK);
gen_and(b0, b1);
b0 = b1;
}
