On Thu, Jul 06, 2023 at 11:57:06AM +0200, Jesper Dangaard Brouer wrote:
>
> On 05/07/2023 19.25, Stanislav Fomichev wrote:
> > On 07/03, Larysa Zaremba wrote:
> > > In order to test VLAN tag and checksum level XDP hints in
> > > hardware-independent selfttests, implement newly added XDP hints in veth
> > > driver.
> > >
> > > Signed-off-by: Larysa Zaremba <larysa.zaremba@xxxxxxxxx>
> > > ---
> > > drivers/net/veth.c | 40 ++++++++++++++++++++++++++++++++++++++++
> > > 1 file changed, 40 insertions(+)
> > >
> > > diff --git a/drivers/net/veth.c b/drivers/net/veth.c
> > > index 614f3e3efab0..a7f2b679551d 100644
> > > --- a/drivers/net/veth.c
> > > +++ b/drivers/net/veth.c
> > > @@ -1732,6 +1732,44 @@ static int veth_xdp_rx_hash(const struct xdp_md *ctx, u32 *hash,
> > > return 0;
> > > }
> > > +static int veth_xdp_rx_vlan_tag(const struct xdp_md *ctx, u16 *vlan_tag,
> > > + __be16 *vlan_proto)
> > > +{
> > > + struct veth_xdp_buff *_ctx = (void *)ctx;
> > > + struct sk_buff *skb = _ctx->skb;
> > > + int err;
> > > +
> > > + if (!skb)
> > > + return -ENODATA;
> > > +
> >
> > [..]
> >
> > > + err = __vlan_hwaccel_get_tag(skb, vlan_tag);
> >
> > We probably need to open code __vlan_hwaccel_get_tag here. Because it
> > returns -EINVAL on !skb_vlan_tag_present where the expectation, for us,
> > I'm assuming is -ENODATA?
> >
>
> Looking at in-tree users of __vlan_hwaccel_get_tag(), they don't use the
> err value for anything. Thus, we can just change
> __vlan_hwaccel_get_tag() to return -ENODATA instead of -EINVAL. (And
> also remember __vlan_get_tag() adjustmment).
>
Seems like a good idea, will include those changes it in v3.
>
> $ git diff
> diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
> index 6ba71957851e..fb35d7dd77a2 100644
> --- a/include/linux/if_vlan.h
> +++ b/include/linux/if_vlan.h
> @@ -540,7 +540,7 @@ static inline int __vlan_get_tag(const struct sk_buff
> *skb, u16 *vlan_tci)
> struct vlan_ethhdr *veth = skb_vlan_eth_hdr(skb);
>
> if (!eth_type_vlan(veth->h_vlan_proto))
> - return -EINVAL;
> + return -ENODATA;
>
> *vlan_tci = ntohs(veth->h_vlan_TCI);
> return 0;
> @@ -561,7 +561,7 @@ static inline int __vlan_hwaccel_get_tag(const struct
> sk_buff *skb,
> return 0;
> } else {
> *vlan_tci = 0;
> - return -EINVAL;
> + return -ENODATA;
> }
> }
>
>
>
> > > + if (err)
> > > + return err;
> > > +
> > > + *vlan_proto = skb->vlan_proto;
> > > + return err;
> > > +}
> > > +
> > > +static int veth_xdp_rx_csum_lvl(const struct xdp_md *ctx, u8 *csum_level)
> > > +{
> > > + struct veth_xdp_buff *_ctx = (void *)ctx;
> > > + struct sk_buff *skb = _ctx->skb;
> > > +
> > > + if (!skb)
> > > + return -ENODATA;
> > > +
> > > + if (skb->ip_summed == CHECKSUM_UNNECESSARY)
> > > + *csum_level = skb->csum_level;
> > > + else if (skb->ip_summed == CHECKSUM_PARTIAL &&
> > > + skb_checksum_start_offset(skb) == skb_transport_offset(skb) ||
> > > + skb->csum_valid)
> > > + *csum_level = 0;
> > > + else
> > > + return -ENODATA;
> > > +
> > > + return 0;
> > > +}
> > > +
> [...]
>
