Netdev Archive on lore.kernel.org
help / color / mirror / Atom feed
From: "Li,Rongqing" <lirongqing@baidu.com>
To: Magnus Karlsson <magnus.karlsson@gmail.com>
Cc: "Network Development" <netdev@vger.kernel.org>,
	intel-wired-lan <intel-wired-lan@lists.osuosl.org>,
	"Karlsson, Magnus" <magnus.karlsson@intel.com>,
	"Björn Töpel" <bjorn.topel@intel.com>
Subject: 答复: [Intel-wired-lan] [PATCH 1/2] xdp: i40e: ixgbe: ixgbevf: not flip rx buffer for copy mode xdp
Date: Tue, 21 Jul 2020 01:42:39 +0000	[thread overview]
Message-ID: <7b87919a454c4c7ba3d431783069e686@baidu.com> (raw)
In-Reply-To: <CAJ8uoz2hdemss9S5vuF=Ttapkfb8U4YJy41oVjpMUVLiCOJTkw@mail.gmail.com>



> -----邮件原件-----
> 发件人: Magnus Karlsson [mailto:magnus.karlsson@gmail.com]
> 发送时间: 2020年7月20日 15:21
> 收件人: Li,Rongqing <lirongqing@baidu.com>
> 抄送: Network Development <netdev@vger.kernel.org>; intel-wired-lan
> <intel-wired-lan@lists.osuosl.org>; Karlsson, Magnus
> <magnus.karlsson@intel.com>; Björn Töpel <bjorn.topel@intel.com>
> 主题: Re: [Intel-wired-lan] [PATCH 1/2] xdp: i40e: ixgbe: ixgbevf: not flip rx
> buffer for copy mode xdp
> 
> On Fri, Jul 17, 2020 at 8:24 AM Li RongQing <lirongqing@baidu.com> wrote:
> >
> > i40e/ixgbe/ixgbevf_rx_buffer_flip in copy mode xdp can lead to data
> > corruption, like the following flow:
> >
> >    1. first skb is not for xsk, and forwarded to another device
> >       or socket queue
> >    2. seconds skb is for xsk, copy data to xsk memory, and page
> >       of skb->data is released
> >    3. rx_buff is reusable since only first skb is in it, but
> >       *_rx_buffer_flip will make that page_offset is set to
> >       first skb data
> >    4. then reuse rx buffer, first skb which still is living
> >       will be corrupted.
e, but known size type */
> >         u32 id;
> > @@ -73,6 +75,7 @@ struct xdp_buff {
> >         struct xdp_rxq_info *rxq;
> >         struct xdp_txq_info *txq;
> >         u32 frame_sz; /* frame size to deduce data_hard_end/reserved
> > tailroom*/
> > +       u32 flags;
> 
> RongQing,
> 
> Sorry that I was not clear enough. Could you please submit the simple patch
> you had, the one that only tests for the memory type.
> 
> if (xdp->rxq->mem.type != MEM_TYPE_XSK_BUFF_POOL)
>       i40e_rx_buffer_flip(rx_ring, rx_buffer, size);
> 
> I do not think that adding a flags field in the xdp_mem_info to fix an Intel driver
> problem will be hugely popular. The struct is also meant to contain long lived
> information, not things that will frequently change.
> 


Thank you Magnus

My original suggestion is wrong , it should be following

if (xdp->rxq->mem.type == MEM_TYPE_XSK_BUFF_POOL)
       i40e_rx_buffer_flip(rx_ring, rx_buffer, size);


but I feel it is not enough to only check mem.type, it must ensure that map_type is BPF_MAP_TYPE_XSKMAP ? but it is not expose. 

other maptype, like BPF_MAP_TYPE_DEVMAP,  and if mem.type is MEM_TYPE_PAGE_SHARED, not flip the rx buffer, will cause data corruption.


-Li 




  reply	other threads:[~2020-07-21  1:58 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-17  6:24 [PATCH 0/2] intel/xdp fixes for fliping rx buffer Li RongQing
2020-07-17  6:24 ` [PATCH 1/2] xdp: i40e: ixgbe: ixgbevf: not flip rx buffer for copy mode xdp Li RongQing
2020-07-20  7:21   ` [Intel-wired-lan] " Magnus Karlsson
2020-07-21  1:42     ` Li,Rongqing [this message]
2020-07-21  7:49     ` 答复: " Li,Rongqing
2020-07-17  6:24 ` [PATCH 2/2] ice/xdp: not adjust " Li RongQing
2020-08-18 14:04 ` [Intel-wired-lan] [PATCH 0/2] intel/xdp fixes for fliping rx buffer Björn Töpel
2020-08-19  1:37   ` 答复: " Li,Rongqing
2020-08-19  6:44     ` Björn Töpel
2020-08-19  8:17       ` 答复: " Li,Rongqing
2020-08-19  8:31         ` Björn Töpel
2020-08-19  8:52           ` Björn Töpel
2020-08-20 15:13   ` Björn Töpel
2020-08-20 16:51     ` Maciej Fijalkowski
2020-08-20 18:04       ` Björn Töpel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7b87919a454c4c7ba3d431783069e686@baidu.com \
    --to=lirongqing@baidu.com \
    --cc=bjorn.topel@intel.com \
    --cc=intel-wired-lan@lists.osuosl.org \
    --cc=magnus.karlsson@gmail.com \
    --cc=magnus.karlsson@intel.com \
    --cc=netdev@vger.kernel.org \
    --subject='Re: 答复: [Intel-wired-lan] [PATCH 1/2] xdp: i40e: ixgbe: ixgbevf: not flip rx buffer for copy mode xdp' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).