diff mbox series

[bpf-next,v2,17/20] veth: Implement VLAN tag and checksum level XDP hint

Message ID 20230703181226.19380-18-larysa.zaremba@intel.com (mailing list archive)
State Changes Requested
Delegated to: BPF
Headers show
Series XDP metadata via kfuncs for ice | expand

Checks

Context Check Description
bpf/vmtest-bpf-next-PR success PR summary
bpf/vmtest-bpf-next-VM_Test-1 success Logs for ${{ matrix.test }} on ${{ matrix.arch }} with ${{ matrix.toolchain_full }}
bpf/vmtest-bpf-next-VM_Test-2 success Logs for ShellCheck
bpf/vmtest-bpf-next-VM_Test-3 success Logs for build for aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-4 success Logs for build for s390x with gcc
bpf/vmtest-bpf-next-VM_Test-5 success Logs for build for x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-6 fail Logs for build for x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-7 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-8 success Logs for veristat
netdev/series_format fail Series longer than 15 patches (and no cover letter)
netdev/tree_selection success Clearly marked for bpf-next, async
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit fail Errors and warnings before: 8 this patch: 9
netdev/cc_maintainers warning 4 maintainers not CCed: hawk@kernel.org edumazet@google.com davem@davemloft.net pabeni@redhat.com
netdev/build_clang fail Errors and warnings before: 23 this patch: 21
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn fail Errors and warnings before: 8 this patch: 9
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 52 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Larysa Zaremba July 3, 2023, 6:12 p.m. UTC 
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@intel.com>
---
 drivers/net/veth.c | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

Comments

Stanislav Fomichev July 5, 2023, 5:25 p.m. UTC | #1 
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@intel.com>
> ---
>  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?

> +	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;
> +}
> +
>  static const struct net_device_ops veth_netdev_ops = {
>  	.ndo_init            = veth_dev_init,
>  	.ndo_open            = veth_open,
> @@ -1756,6 +1794,8 @@ static const struct net_device_ops veth_netdev_ops = {
>  static const struct xdp_metadata_ops veth_xdp_metadata_ops = {
>  	.xmo_rx_timestamp		= veth_xdp_rx_timestamp,
>  	.xmo_rx_hash			= veth_xdp_rx_hash,
> +	.xmo_rx_vlan_tag		= veth_xdp_rx_vlan_tag,
> +	.xmo_rx_csum_lvl		= veth_xdp_rx_csum_lvl,
>  };
>  
>  #define VETH_FEATURES (NETIF_F_SG | NETIF_F_FRAGLIST | NETIF_F_HW_CSUM | \
> -- 
> 2.41.0
>
Jesper Dangaard Brouer July 6, 2023, 9:57 a.m. UTC | #2 
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@intel.com>
>> ---
>>   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).


$ 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;
>> +}
>> +
[...]
Larysa Zaremba July 6, 2023, 10:15 a.m. UTC | #3 
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@intel.com>
> > > ---
> > >   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;
> > > +}
> > > +
> [...]
>
diff mbox series

Patch

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);
+	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;
+}
+
 static const struct net_device_ops veth_netdev_ops = {
 	.ndo_init            = veth_dev_init,
 	.ndo_open            = veth_open,
@@ -1756,6 +1794,8 @@  static const struct net_device_ops veth_netdev_ops = {
 static const struct xdp_metadata_ops veth_xdp_metadata_ops = {
 	.xmo_rx_timestamp		= veth_xdp_rx_timestamp,
 	.xmo_rx_hash			= veth_xdp_rx_hash,
+	.xmo_rx_vlan_tag		= veth_xdp_rx_vlan_tag,
+	.xmo_rx_csum_lvl		= veth_xdp_rx_csum_lvl,
 };
 
 #define VETH_FEATURES (NETIF_F_SG | NETIF_F_FRAGLIST | NETIF_F_HW_CSUM | \