Re: [PATCH nft] proto: fix VLAN header definition

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 27.11, Florian Westphal wrote:
> Florian Westphal <fw@xxxxxxxxx> wrote:
> > Patrick McHardy <kaber@xxxxxxxxx> wrote:
> > > On 27.11, Florian Westphal wrote:
> > > > Patrick McHardy <kaber@xxxxxxxxx> wrote:
> > > > > The VID is located after Priority and CFI.
> > > > 
> > > > With this patch matching on vlan id does not work for me anymore
> > > > on x86-64.
> > > > 
> > > > With trace-patch nft but without this patch:
> > > > 
> > > > table bridge filter {
> > > > chain input {
> > > >    type filter hook input priority -200; policy accept;
> > > >    vlan id 4094 counter packets 827 bytes 63839
> > > > 
> > > > With this patch, the counters remain at zero:
> > > > 
> > > > unknown unknown & 0xfff [invalid type] == 0xffe [invalid type] counter packets 850 bytes 65375
> > > > vlan id 4094 counter packets 0 bytes 0
> > > > 
> > > > (The 'unknown unknown' line is the 'old' vlan rule added by unpatched
> > > >  nft binary, the 'vlan id' is the one added with the patched one).
> > > 
> > > Odd, since it decodes correctly. Could you send the output of
> > > nft --debug=netlink with and without the patch?
> > 
> > master:
> > nft --debug=netlink  add rule  bridge filter input vlan id 4094 counter
> > bridge filter input
> >  [ payload load 2b @ link header + 12 => reg 1 ]
> >  [ cmp eq reg 1 0x00000081 ]
> >  [ payload load 2b @ link header + 14 => reg 1 ]
> >  [ bitwise reg 1 = (reg=1 & 0x0000ff0f ) ^ 0x00000000 ]
> >  [ cmp eq reg 1 0x0000fe0f ]
> >  [ counter pkts 0 bytes 0 ]
> 
> I checked nft_payload again and I believe rebuild of the vlan header is
> correct (a bug there would also explain this problem).

Yes, I also did that and it looks correct. I think we probably have a
discrepancy with bit numbering:

Looking at an older patch of you:

-               [IPHDR_VERSION]         = HDR_BITFIELD("version", &integer_type, 0, 4),
-               [IPHDR_HDRLENGTH]       = HDR_BITFIELD("hdrlength", &integer_type, 4, 4),
+               [IPHDR_VERSION]         = HDR_BITFIELD("version", &integer_type, 4, 4),
+               [IPHDR_HDRLENGTH]       = HDR_BITFIELD("hdrlength", &integer_type, 0, 4),

So you seem to assume a numbering which corresponds to how you would express
it in C. My patch assumes numbering as used in the RFCs/IEEE standards, which
is basically the opposite direction.

I have to check that code in more detail.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Netfitler Users]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux