* [PATCH net] ixgbe: xsk: fix for metasize when construct skb by xdp_buff
@ 2021-06-09 12:22 ` Xuan Zhuo
0 siblings, 0 replies; 5+ messages in thread
From: Xuan Zhuo @ 2021-06-09 12:22 UTC (permalink / raw)
To: netdev
Cc: Jesse Brandeburg, Tony Nguyen, David S. Miller, Jakub Kicinski,
Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer,
John Fastabend, Jeff Kirsher, Björn Töpel,
intel-wired-lan, bpf
We should copy data_meta to the skb space. Then use __skb_pull to
correct skb->data
Fixes: d0bcacd0a1309 ("ixgbe: add AF_XDP zero-copy Rx support")
Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
index f72d2978263b..ee88107fa57a 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
@@ -204,7 +204,7 @@ static struct sk_buff *ixgbe_construct_skb_zc(struct ixgbe_ring *rx_ring,
struct ixgbe_rx_buffer *bi)
{
unsigned int metasize = bi->xdp->data - bi->xdp->data_meta;
- unsigned int datasize = bi->xdp->data_end - bi->xdp->data;
+ unsigned int datasize = bi->xdp->data_end - bi->xdp->data_meta;
struct sk_buff *skb;
/* allocate a skb to store the frags */
@@ -214,10 +214,12 @@ static struct sk_buff *ixgbe_construct_skb_zc(struct ixgbe_ring *rx_ring,
if (unlikely(!skb))
return NULL;
- skb_reserve(skb, bi->xdp->data - bi->xdp->data_hard_start);
- memcpy(__skb_put(skb, datasize), bi->xdp->data, datasize);
- if (metasize)
+ skb_reserve(skb, bi->xdp->data_meta - bi->xdp->data_hard_start);
+ memcpy(__skb_put(skb, datasize), bi->xdp->data_meta, datasize);
+ if (metasize) {
+ __skb_pull(skb, metasize);
skb_metadata_set(skb, metasize);
+ }
xsk_buff_free(bi->xdp);
bi->xdp = NULL;
--
2.31.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [Intel-wired-lan] [PATCH net] ixgbe: xsk: fix for metasize when construct skb by xdp_buff
@ 2021-06-09 12:22 ` Xuan Zhuo
0 siblings, 0 replies; 5+ messages in thread
From: Xuan Zhuo @ 2021-06-09 12:22 UTC (permalink / raw)
To: intel-wired-lan
We should copy data_meta to the skb space. Then use __skb_pull to
correct skb->data
Fixes: d0bcacd0a1309 ("ixgbe: add AF_XDP zero-copy Rx support")
Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
index f72d2978263b..ee88107fa57a 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
@@ -204,7 +204,7 @@ static struct sk_buff *ixgbe_construct_skb_zc(struct ixgbe_ring *rx_ring,
struct ixgbe_rx_buffer *bi)
{
unsigned int metasize = bi->xdp->data - bi->xdp->data_meta;
- unsigned int datasize = bi->xdp->data_end - bi->xdp->data;
+ unsigned int datasize = bi->xdp->data_end - bi->xdp->data_meta;
struct sk_buff *skb;
/* allocate a skb to store the frags */
@@ -214,10 +214,12 @@ static struct sk_buff *ixgbe_construct_skb_zc(struct ixgbe_ring *rx_ring,
if (unlikely(!skb))
return NULL;
- skb_reserve(skb, bi->xdp->data - bi->xdp->data_hard_start);
- memcpy(__skb_put(skb, datasize), bi->xdp->data, datasize);
- if (metasize)
+ skb_reserve(skb, bi->xdp->data_meta - bi->xdp->data_hard_start);
+ memcpy(__skb_put(skb, datasize), bi->xdp->data_meta, datasize);
+ if (metasize) {
+ __skb_pull(skb, metasize);
skb_metadata_set(skb, metasize);
+ }
xsk_buff_free(bi->xdp);
bi->xdp = NULL;
--
2.31.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH net] ixgbe: xsk: fix for metasize when construct skb by xdp_buff
2021-06-09 12:22 ` [Intel-wired-lan] " Xuan Zhuo
@ 2021-06-11 15:12 ` Maciej Fijalkowski
-1 siblings, 0 replies; 5+ messages in thread
From: Maciej Fijalkowski @ 2021-06-11 15:12 UTC (permalink / raw)
To: Xuan Zhuo
Cc: netdev, Jesse Brandeburg, Tony Nguyen, David S. Miller,
Jakub Kicinski, Alexei Starovoitov, Daniel Borkmann,
Jesper Dangaard Brouer, John Fastabend, Jeff Kirsher,
Björn Töpel, intel-wired-lan, bpf
On Wed, Jun 09, 2021 at 08:22:44PM +0800, Xuan Zhuo wrote:
> We should copy data_meta to the skb space. Then use __skb_pull to
> correct skb->data
This looks like a bug that has been sitting over here for sometime. Have
you encountered this during your virtio-net's AF_XDP ZC work? I'm all ears
how you spotted this.
Anyway, other drivers needs such fixing too. Are you willing to do that or
should we take it on our side?
Magnus is OOO today, I'd like to get his final ack on that.
From me:
Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Thanks!
>
> Fixes: d0bcacd0a1309 ("ixgbe: add AF_XDP zero-copy Rx support")
> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> ---
> drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
> index f72d2978263b..ee88107fa57a 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
> @@ -204,7 +204,7 @@ static struct sk_buff *ixgbe_construct_skb_zc(struct ixgbe_ring *rx_ring,
> struct ixgbe_rx_buffer *bi)
> {
> unsigned int metasize = bi->xdp->data - bi->xdp->data_meta;
> - unsigned int datasize = bi->xdp->data_end - bi->xdp->data;
> + unsigned int datasize = bi->xdp->data_end - bi->xdp->data_meta;
> struct sk_buff *skb;
>
> /* allocate a skb to store the frags */
> @@ -214,10 +214,12 @@ static struct sk_buff *ixgbe_construct_skb_zc(struct ixgbe_ring *rx_ring,
> if (unlikely(!skb))
> return NULL;
>
> - skb_reserve(skb, bi->xdp->data - bi->xdp->data_hard_start);
> - memcpy(__skb_put(skb, datasize), bi->xdp->data, datasize);
> - if (metasize)
> + skb_reserve(skb, bi->xdp->data_meta - bi->xdp->data_hard_start);
> + memcpy(__skb_put(skb, datasize), bi->xdp->data_meta, datasize);
> + if (metasize) {
> + __skb_pull(skb, metasize);
> skb_metadata_set(skb, metasize);
> + }
>
> xsk_buff_free(bi->xdp);
> bi->xdp = NULL;
> --
> 2.31.0
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Intel-wired-lan] [PATCH net] ixgbe: xsk: fix for metasize when construct skb by xdp_buff
@ 2021-06-11 15:12 ` Maciej Fijalkowski
0 siblings, 0 replies; 5+ messages in thread
From: Maciej Fijalkowski @ 2021-06-11 15:12 UTC (permalink / raw)
To: intel-wired-lan
On Wed, Jun 09, 2021 at 08:22:44PM +0800, Xuan Zhuo wrote:
> We should copy data_meta to the skb space. Then use __skb_pull to
> correct skb->data
This looks like a bug that has been sitting over here for sometime. Have
you encountered this during your virtio-net's AF_XDP ZC work? I'm all ears
how you spotted this.
Anyway, other drivers needs such fixing too. Are you willing to do that or
should we take it on our side?
Magnus is OOO today, I'd like to get his final ack on that.
From me:
Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Thanks!
>
> Fixes: d0bcacd0a1309 ("ixgbe: add AF_XDP zero-copy Rx support")
> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> ---
> drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
> index f72d2978263b..ee88107fa57a 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
> @@ -204,7 +204,7 @@ static struct sk_buff *ixgbe_construct_skb_zc(struct ixgbe_ring *rx_ring,
> struct ixgbe_rx_buffer *bi)
> {
> unsigned int metasize = bi->xdp->data - bi->xdp->data_meta;
> - unsigned int datasize = bi->xdp->data_end - bi->xdp->data;
> + unsigned int datasize = bi->xdp->data_end - bi->xdp->data_meta;
> struct sk_buff *skb;
>
> /* allocate a skb to store the frags */
> @@ -214,10 +214,12 @@ static struct sk_buff *ixgbe_construct_skb_zc(struct ixgbe_ring *rx_ring,
> if (unlikely(!skb))
> return NULL;
>
> - skb_reserve(skb, bi->xdp->data - bi->xdp->data_hard_start);
> - memcpy(__skb_put(skb, datasize), bi->xdp->data, datasize);
> - if (metasize)
> + skb_reserve(skb, bi->xdp->data_meta - bi->xdp->data_hard_start);
> + memcpy(__skb_put(skb, datasize), bi->xdp->data_meta, datasize);
> + if (metasize) {
> + __skb_pull(skb, metasize);
> skb_metadata_set(skb, metasize);
> + }
>
> xsk_buff_free(bi->xdp);
> bi->xdp = NULL;
> --
> 2.31.0
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Intel-wired-lan] [PATCH net] ixgbe: xsk: fix for metasize when construct skb by xdp_buff
2021-06-11 15:12 ` [Intel-wired-lan] " Maciej Fijalkowski
(?)
@ 2021-06-15 3:39 ` Xuan Zhuo
-1 siblings, 0 replies; 5+ messages in thread
From: Xuan Zhuo @ 2021-06-15 3:39 UTC (permalink / raw)
To: intel-wired-lan
On Fri, 11 Jun 2021 17:12:45 +0200, Maciej Fijalkowski <maciej.fijalkowski@intel.com> wrote:
> On Wed, Jun 09, 2021 at 08:22:44PM +0800, Xuan Zhuo wrote:
> > We should copy data_meta to the skb space. Then use __skb_pull to
> > correct skb->data
>
> This looks like a bug that has been sitting over here for sometime. Have
> you encountered this during your virtio-net's AF_XDP ZC work? I'm all ears
> how you spotted this.
>
> Anyway, other drivers needs such fixing too. Are you willing to do that or
> should we take it on our side?
You are right, I didn't pay attention to other drivers before. I have already
submitted the patch.
[PATCH net] xdp, net: fix for construct skb by xdp inside xsk zc rx
Thanks!
>
> Magnus is OOO today, I'd like to get his final ack on that.
>
> From me:
> Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
>
> Thanks!
>
> >
> > Fixes: d0bcacd0a1309 ("ixgbe: add AF_XDP zero-copy Rx support")
> > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > ---
> > drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c | 10 ++++++----
> > 1 file changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
> > index f72d2978263b..ee88107fa57a 100644
> > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
> > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
> > @@ -204,7 +204,7 @@ static struct sk_buff *ixgbe_construct_skb_zc(struct ixgbe_ring *rx_ring,
> > struct ixgbe_rx_buffer *bi)
> > {
> > unsigned int metasize = bi->xdp->data - bi->xdp->data_meta;
> > - unsigned int datasize = bi->xdp->data_end - bi->xdp->data;
> > + unsigned int datasize = bi->xdp->data_end - bi->xdp->data_meta;
> > struct sk_buff *skb;
> >
> > /* allocate a skb to store the frags */
> > @@ -214,10 +214,12 @@ static struct sk_buff *ixgbe_construct_skb_zc(struct ixgbe_ring *rx_ring,
> > if (unlikely(!skb))
> > return NULL;
> >
> > - skb_reserve(skb, bi->xdp->data - bi->xdp->data_hard_start);
> > - memcpy(__skb_put(skb, datasize), bi->xdp->data, datasize);
> > - if (metasize)
> > + skb_reserve(skb, bi->xdp->data_meta - bi->xdp->data_hard_start);
> > + memcpy(__skb_put(skb, datasize), bi->xdp->data_meta, datasize);
> > + if (metasize) {
> > + __skb_pull(skb, metasize);
> > skb_metadata_set(skb, metasize);
> > + }
> >
> > xsk_buff_free(bi->xdp);
> > bi->xdp = NULL;
> > --
> > 2.31.0
> >
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-06-15 3:39 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-09 12:22 [PATCH net] ixgbe: xsk: fix for metasize when construct skb by xdp_buff Xuan Zhuo
2021-06-09 12:22 ` [Intel-wired-lan] " Xuan Zhuo
2021-06-11 15:12 ` Maciej Fijalkowski
2021-06-11 15:12 ` [Intel-wired-lan] " Maciej Fijalkowski
2021-06-15 3:39 ` Xuan Zhuo
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.