On Mon, Jan 13, 2020 at 06:50:50PM +0800, Tonghao Zhang wrote:
> On Mon, Jan 13, 2020 at 6:03 PM Simon Horman <[email protected]>
> wrote:
> >
> > Hi,
> >
> > On Mon, Jan 13, 2020 at 05:28:04PM +0800, Tonghao Zhang wrote:
> > > On Fri, Jan 10, 2020 at 7:06 PM Simon Horman <[email protected]>
> > > wrote:
> > > >
> > > > From: John Hurley <[email protected]>
> > > >
> > > > Openstack may set an skb mark of 0 in tunnel rules. This is considered
> > > > to
> > > > be an unused/unset value. However, it prevents the rule from being
> > > > offloaded.
> > > >
> > > > Check if the key value of the skb mark is 0 when it is in use (mask is
> > > > set to all ones). If it is then ignore the field and continue with TC
> > > > offload.
> > > >
> > > > Signed-off-by: John Hurley <[email protected]>
> > > > [simon; check for exact-match rather than any match]
> > > > Signed-off-by: Simon Horman <[email protected]>
> > > > ---
> > > > lib/netdev-offload-tc.c | 5 +++++
> > > > 1 file changed, 5 insertions(+)
> > > >
> > > > diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
> > > > index 7453078d535f..daff8a379e97 100644
> > > > --- a/lib/netdev-offload-tc.c
> > > > +++ b/lib/netdev-offload-tc.c
> > > > @@ -1619,6 +1619,11 @@ netdev_tc_flow_put(struct netdev *netdev, struct
> > > > match *match,
> > > > mask->ct_label = OVS_U128_ZERO;
> > > > }
> > > >
> > > > + /* ignore exact match on skb_mark of 0. */
> > > > + if (mask->pkt_mark == UINT32_MAX && !key->pkt_mark) {
> > > > + mask->pkt_mark = 0;
> > > > + }
> > > Why not: if (!(mask->pkt_mark & key->pkt_mark))
> >
> > Its not clear to me that only returns true in the case
> > where there is an exact match on pkt_mark 0. Which is the case
> > that is considered to be an unused/unset value from a HW offload
> > perspective.
> if mask->pkt_mark & key->pkt_mark == 0, the HW offload will return error ?
> if so, there is a case that key->pkt_mark != 0, but mask->pkt_mark &
> key->pkt_mark == 0.
I think there are some subtle issues here.
As it stands HW offload support in the Linux kernel does not support
matching pkt_mark. So there is actually no way to express such a match
and request the Kernel that it be offloaded. Instead, ovs-vswitchd
rejects such flows.
The intention of this patch is to add an exception, whereby a match on
pkt_mark 0 is considered to be the same as no match on the pkt_mark.
The reasoning is that on the wire pkt_mark does not exist. And that when
a packet arrives the network stack sets a default value of 0. So in
a sense packets on the wire have this default value. As do those in
a datapath in a NIC which is not pkt_mark aware.
So we can think of the pkt_mark in the HW datapath as being 0.
And I believe your suggestion that as 0 anded with any mask is 0
then we can do so with any mask. I agree to some extent but I am concerned
about forwards compatibility.
In theory a HW datapath may become pkt_mark aware. For example a BPF
program might run in the HW before the OVS datapath and the BPF program
may set the pkt_mark to a non-zero value. I believe that in this
case the change you are suggesting would lead to incorrect behaviour.
So I think that it is best to keep a very restrictive test for this
exception to avoid problems in future.
> > > > err = test_key_and_mask(match);
> > > > if (err) {
> > > > return err;
> > > > --
> > > > 2.20.1
> > > >
> > > > _______________________________________________
> > > > dev mailing list
> > > > [email protected]
> > > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
