diff mbox series

[v3,2/9] virtio-net: set up xdp for multi buffer packets

Message ID 20230103064012.108029-3-hengqi@linux.alibaba.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series virtio-net: support multi buffer xdp | expand

Checks

Context Check Description
netdev/tree_selection success Guessed tree name to be net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix warning Target tree name not specified in the subject
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers warning 2 maintainers not CCed: virtualization@lists.linux-foundation.org hawk@kernel.org
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch warning WARNING: line length of 88 exceeds 80 columns WARNING: line length of 90 exceeds 80 columns
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Heng Qi Jan. 3, 2023, 6:40 a.m. UTC
When the xdp program sets xdp.frags, which means it can process
multi-buffer packets over larger MTU, so we continue to support xdp.
But for single-buffer xdp, we should keep checking for MTU.

Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
Reviewed-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
 drivers/net/virtio_net.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

Comments

Heng Qi Jan. 9, 2023, 2:48 a.m. UTC | #1
在 2023/1/3 下午2:40, Heng Qi 写道:
> When the xdp program sets xdp.frags, which means it can process
> multi-buffer packets over larger MTU, so we continue to support xdp.
> But for single-buffer xdp, we should keep checking for MTU.
>
> Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
> Reviewed-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> ---
>   drivers/net/virtio_net.c | 10 ++++++----
>   1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 443aa7b8f0ad..60e199811212 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -3074,7 +3074,9 @@ static int virtnet_restore_guest_offloads(struct virtnet_info *vi)
>   static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog,
>   			   struct netlink_ext_ack *extack)
>   {
> -	unsigned long int max_sz = PAGE_SIZE - sizeof(struct padded_vnet_hdr);
> +	unsigned int room = SKB_DATA_ALIGN(VIRTIO_XDP_HEADROOM +
> +					   sizeof(struct skb_shared_info));
> +	unsigned int max_sz = PAGE_SIZE - room - ETH_HLEN;

Hi Jason, I've updated the calculation of 'max_sz' in this patch instead 
of a separate bugfix, since doing so also seemed clear.

Thanks.

>   	struct virtnet_info *vi = netdev_priv(dev);
>   	struct bpf_prog *old_prog;
>   	u16 xdp_qp = 0, curr_qp;
> @@ -3095,9 +3097,9 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog,
>   		return -EINVAL;
>   	}
>   
> -	if (dev->mtu > max_sz) {
> -		NL_SET_ERR_MSG_MOD(extack, "MTU too large to enable XDP");
> -		netdev_warn(dev, "XDP requires MTU less than %lu\n", max_sz);
> +	if (prog && !prog->aux->xdp_has_frags && dev->mtu > max_sz) {
> +		NL_SET_ERR_MSG_MOD(extack, "MTU too large to enable XDP without frags");
> +		netdev_warn(dev, "single-buffer XDP requires MTU less than %u\n", max_sz);
>   		return -EINVAL;
>   	}
>
Jason Wang Jan. 9, 2023, 8:56 a.m. UTC | #2
On Mon, Jan 9, 2023 at 10:48 AM Heng Qi <hengqi@linux.alibaba.com> wrote:
>
>
>
> 在 2023/1/3 下午2:40, Heng Qi 写道:
> > When the xdp program sets xdp.frags, which means it can process
> > multi-buffer packets over larger MTU, so we continue to support xdp.
> > But for single-buffer xdp, we should keep checking for MTU.
> >
> > Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
> > Reviewed-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > ---
> >   drivers/net/virtio_net.c | 10 ++++++----
> >   1 file changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 443aa7b8f0ad..60e199811212 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -3074,7 +3074,9 @@ static int virtnet_restore_guest_offloads(struct virtnet_info *vi)
> >   static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog,
> >                          struct netlink_ext_ack *extack)
> >   {
> > -     unsigned long int max_sz = PAGE_SIZE - sizeof(struct padded_vnet_hdr);
> > +     unsigned int room = SKB_DATA_ALIGN(VIRTIO_XDP_HEADROOM +
> > +                                        sizeof(struct skb_shared_info));
> > +     unsigned int max_sz = PAGE_SIZE - room - ETH_HLEN;
>
> Hi Jason, I've updated the calculation of 'max_sz' in this patch instead
> of a separate bugfix, since doing so also seemed clear.

Sure, I will review it with this series no later than the end of this week.

Thanks

>
> Thanks.
>
> >       struct virtnet_info *vi = netdev_priv(dev);
> >       struct bpf_prog *old_prog;
> >       u16 xdp_qp = 0, curr_qp;
> > @@ -3095,9 +3097,9 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog,
> >               return -EINVAL;
> >       }
> >
> > -     if (dev->mtu > max_sz) {
> > -             NL_SET_ERR_MSG_MOD(extack, "MTU too large to enable XDP");
> > -             netdev_warn(dev, "XDP requires MTU less than %lu\n", max_sz);
> > +     if (prog && !prog->aux->xdp_has_frags && dev->mtu > max_sz) {
> > +             NL_SET_ERR_MSG_MOD(extack, "MTU too large to enable XDP without frags");
> > +             netdev_warn(dev, "single-buffer XDP requires MTU less than %u\n", max_sz);
> >               return -EINVAL;
> >       }
> >
>
Jason Wang Jan. 13, 2023, 2:49 a.m. UTC | #3
在 2023/1/3 14:40, Heng Qi 写道:
> When the xdp program sets xdp.frags, which means it can process
> multi-buffer packets over larger MTU, so we continue to support xdp.
> But for single-buffer xdp, we should keep checking for MTU.
>
> Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
> Reviewed-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> ---
>   drivers/net/virtio_net.c | 10 ++++++----
>   1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 443aa7b8f0ad..60e199811212 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -3074,7 +3074,9 @@ static int virtnet_restore_guest_offloads(struct virtnet_info *vi)
>   static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog,
>   			   struct netlink_ext_ack *extack)
>   {
> -	unsigned long int max_sz = PAGE_SIZE - sizeof(struct padded_vnet_hdr);
> +	unsigned int room = SKB_DATA_ALIGN(VIRTIO_XDP_HEADROOM +
> +					   sizeof(struct skb_shared_info));
> +	unsigned int max_sz = PAGE_SIZE - room - ETH_HLEN;
>   	struct virtnet_info *vi = netdev_priv(dev);
>   	struct bpf_prog *old_prog;
>   	u16 xdp_qp = 0, curr_qp;
> @@ -3095,9 +3097,9 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog,
>   		return -EINVAL;
>   	}
>   
> -	if (dev->mtu > max_sz) {
> -		NL_SET_ERR_MSG_MOD(extack, "MTU too large to enable XDP");
> -		netdev_warn(dev, "XDP requires MTU less than %lu\n", max_sz);
> +	if (prog && !prog->aux->xdp_has_frags && dev->mtu > max_sz) {
> +		NL_SET_ERR_MSG_MOD(extack, "MTU too large to enable XDP without frags");
> +		netdev_warn(dev, "single-buffer XDP requires MTU less than %u\n", max_sz);
>   		return -EINVAL;
>   	}


I think we probably need to backport this to -stable. So I suggest to 
move/squash the check of !prog->aux->xdp_has_frags to one of the 
following patch.

With this,

Acked-by: Jason Wang <jasowang@redhat.com>

Thanks


>
Heng Qi Jan. 13, 2023, 2:59 a.m. UTC | #4
在 2023/1/13 上午10:49, Jason Wang 写道:
>
> 在 2023/1/3 14:40, Heng Qi 写道:
>> When the xdp program sets xdp.frags, which means it can process
>> multi-buffer packets over larger MTU, so we continue to support xdp.
>> But for single-buffer xdp, we should keep checking for MTU.
>>
>> Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
>> Reviewed-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
>> ---
>>   drivers/net/virtio_net.c | 10 ++++++----
>>   1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> index 443aa7b8f0ad..60e199811212 100644
>> --- a/drivers/net/virtio_net.c
>> +++ b/drivers/net/virtio_net.c
>> @@ -3074,7 +3074,9 @@ static int 
>> virtnet_restore_guest_offloads(struct virtnet_info *vi)
>>   static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog 
>> *prog,
>>                  struct netlink_ext_ack *extack)
>>   {
>> -    unsigned long int max_sz = PAGE_SIZE - sizeof(struct 
>> padded_vnet_hdr);
>> +    unsigned int room = SKB_DATA_ALIGN(VIRTIO_XDP_HEADROOM +
>> +                       sizeof(struct skb_shared_info));
>> +    unsigned int max_sz = PAGE_SIZE - room - ETH_HLEN;
>>       struct virtnet_info *vi = netdev_priv(dev);
>>       struct bpf_prog *old_prog;
>>       u16 xdp_qp = 0, curr_qp;
>> @@ -3095,9 +3097,9 @@ static int virtnet_xdp_set(struct net_device 
>> *dev, struct bpf_prog *prog,
>>           return -EINVAL;
>>       }
>>   -    if (dev->mtu > max_sz) {
>> -        NL_SET_ERR_MSG_MOD(extack, "MTU too large to enable XDP");
>> -        netdev_warn(dev, "XDP requires MTU less than %lu\n", max_sz);
>> +    if (prog && !prog->aux->xdp_has_frags && dev->mtu > max_sz) {
>> +        NL_SET_ERR_MSG_MOD(extack, "MTU too large to enable XDP 
>> without frags");
>> +        netdev_warn(dev, "single-buffer XDP requires MTU less than 
>> %u\n", max_sz);
>>           return -EINVAL;
>>       }
>
>
> I think we probably need to backport this to -stable. So I suggest to 
> move/squash the check of !prog->aux->xdp_has_frags to one of the 
> following patch.

Sure, and you are right.

Thanks.

>
> With this,
>
> Acked-by: Jason Wang <jasowang@redhat.com>
>
> Thanks
>
>
diff mbox series

Patch

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 443aa7b8f0ad..60e199811212 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -3074,7 +3074,9 @@  static int virtnet_restore_guest_offloads(struct virtnet_info *vi)
 static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog,
 			   struct netlink_ext_ack *extack)
 {
-	unsigned long int max_sz = PAGE_SIZE - sizeof(struct padded_vnet_hdr);
+	unsigned int room = SKB_DATA_ALIGN(VIRTIO_XDP_HEADROOM +
+					   sizeof(struct skb_shared_info));
+	unsigned int max_sz = PAGE_SIZE - room - ETH_HLEN;
 	struct virtnet_info *vi = netdev_priv(dev);
 	struct bpf_prog *old_prog;
 	u16 xdp_qp = 0, curr_qp;
@@ -3095,9 +3097,9 @@  static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog,
 		return -EINVAL;
 	}
 
-	if (dev->mtu > max_sz) {
-		NL_SET_ERR_MSG_MOD(extack, "MTU too large to enable XDP");
-		netdev_warn(dev, "XDP requires MTU less than %lu\n", max_sz);
+	if (prog && !prog->aux->xdp_has_frags && dev->mtu > max_sz) {
+		NL_SET_ERR_MSG_MOD(extack, "MTU too large to enable XDP without frags");
+		netdev_warn(dev, "single-buffer XDP requires MTU less than %u\n", max_sz);
 		return -EINVAL;
 	}