Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote:
> On Fri, Nov 27, 2015 at 11:54:17AM +0100, Florian Westphal wrote:
> > Patrick McHardy <kaber@xxxxxxxxx> wrote:
> > > 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.
> >
> > Right, there is a general problem with all sub-byte fields.
> >
> > I just noticed that decoding of ip version/hdrlen doesn't work either.
> > (ip hdrlength 4 ip version 5).
> >
> > I am sure that I tested matching on ip version/hdrlen on both
> > x86-64 and a MSB machine (don't recall architecture, ppc i think).
>
> The existing approach works fine in x86-64 and ppc here.
>
> pahole also reports that version bitfield offset starts at 0, then
> hdrlength starts at 4, both in x86-64 and ppc.
>
> Probably the problem is the way we calculate the shifts. I managed to
> set the offset according to RFCs/IEEE by adjusting the existing
> arithmetics, I think the offset semantics was accidentally changes
> with this first approach to address sub-byte matching.
Thanks for looking at this. I'll take a closer look tomorrow,
your patch works fine for ip version/hdrlength but seems it messes
with endianess somewhere.
With Patricks fix to make VLAN header in IEEE notation:
src/nft --debug=netlink add rule bridge raw prerouting ether type vlan vlan type ip vlan id 4094 ip version 4 counter
bridge raw prerouting
[ payload load 2b @ link header + 12 => reg 1 ]
[ cmp eq reg 1 0x00008100 ]
[ payload load 2b @ link header + 16 => reg 1 ]
[ cmp eq reg 1 0x00000800 ]
[ payload load 2b @ link header + 14 => reg 1 ]
[ bitwise reg 1 = (reg=1 & 0x00000f00 ) ^ 0x00000000 ]
[ cmp eq reg 1 0x00000f00 ]
[ payload load 1b @ network header + 0 => reg 1 ]
[ bitwise reg 1 = (reg=1 & 0x000000f0 ) ^ 0x00000000 ]
[ cmp eq reg 1 0x00000040 ]
[ counter pkts 0 bytes 0 ]
master (counter increments):
# nft --debug=netlink add rule bridge raw prerouting ether type vlan vlan type ip vlan id 4094 ip version 4 counter
bridge raw prerouting
[ payload load 2b @ link header + 12 => reg 1 ]
[ cmp eq reg 1 0x00000081 ]
[ payload load 2b @ link header + 16 => reg 1 ]
[ cmp eq reg 1 0x00000008 ]
[ payload load 2b @ link header + 14 => reg 1 ]
[ bitwise reg 1 = (reg=1 & 0x0000ff0f ) ^ 0x00000000 ]
[ cmp eq reg 1 0x0000fe0f ]
[ payload load 1b @ network header + 0 => reg 1 ]
[ bitwise reg 1 = (reg=1 & 0x000000f0 ) ^ 0x00000000 ]
[ cmp eq reg 1 0x00000040 ]
[ counter pkts 0 bytes 0 ]
--
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
