On Sun, Aug 05, 2018 at 06:10:55AM +0000, Matan Azrad wrote:
> Hi Adrien
>
> From: Adrien Mazarguil
> > Hi Matan,
> >
> > On Thu, Aug 02, 2018 at 05:52:18PM +0000, Matan Azrad wrote:
> > > Hi Adrien
> > >
> > > From: Adrien Mazarguil
> > > > On Thu, Aug 02, 2018 at 10:33:00AM +0000, Matan Azrad wrote:
> > > > > The rte_flow meaning of zero flow mask configuration is to match
> > > > > all the range of the item value.
> > > > > For example, the flow eth / ipv4 dst spec 1.2.3.4 dst mask 0.0.0.0
> > > > > should much all the ipv4 traffic from the rte_flow API perspective.
> > > > >
> > > > > From some kernel perspectives the above rule means to ignore all
> > > > > the
> > > > > ipv4 traffic (e.g. Ubuntu 16.04, 4.15.10).
> > > > >
> > > > > Due to the fact that the tap PMD should provide the rte_flow
> > > > > meaning, it is necessary to ignore the spec in case the mask is
> > > > > zero when it forwards such like flows to the kernel.
> > > > > So, the above rule should be translated to eth / ipv4 to get the
> > > > > correct meaning.
> > > > >
> > > > > Ignore spec configurations when the mask is zero.
> > > >
> > > > I would go further, one should be able to match IP address 0.0.0.0 for
> > instance.
> > > > The PMD should only trust the mask on all fields without looking at
> > > > spec.
> > >
> > > The PMD should convert the RTE flow API to the device configuration,
> > > So I can think on scenarios that the PMD should look on spec.
> >
> > Obviously the PMD needs to take spec into account. What I meant is that for
> > each field, spec must be taken into account according to mask only.
> >
> > For any given field, when mask is empty, don't look at spec, it's like a
> > wildcard.
> > When mask is full, take spec as is, even if spec only contains zeroed bits.
> >
> > User intent in that case is to match a zero value exactly, so it must not
> > result in
> > a wildcard match. If supported, when mask is partial, masked bits are also
> > matched exactly, even if these turn out to be a zero value. Unmasked bits
> > are
> > considered wildcards.
> >
>
> Yes I understand your point Adrien, but I mean that maybe sometimes some spec
> values should be converted to another spec values to get the correct
> translation of rte_flow for a special device.
>
> Here, maybe IP_spec=0.0.0.0 is a special case that should be taken into
> account, so we must validate what's happen in Tap for this case to apply your
> suggestion too, Maybe there was some intentions for spec=0 cases from the
> current code author.
I understand that's a lot of maybes :)
I've checked the code and I'am sure it's a mistake made by the original
author. See tap_flow_create_eth() for instance:
if (!is_zero_ether_addr(&spec->dst)) {
Followed by:
if (!is_zero_ether_addr(&mask->src))
This lack of consistency doesn't make any sense, it cannot be on purpose.
To my credentials I wrote a very similar code which uses TC flower in mlx5
and relies on mask (only) in order to retrieve spec. Have a look at
drivers/net/mlx5/mlx5_nl_flow.c. I validated that traffic where addresses
were all zeroes could be successfully matched.
--
Adrien Mazarguil
6WIND
