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 07:49:00 +0000	[thread overview]
Message-ID: <b3ca06b2292742c39b7362030055869f@baidu.com> (raw)
In-Reply-To: <CAJ8uoz2hdemss9S5vuF=Ttapkfb8U4YJy41oVjpMUVLiCOJTkw@mail.gmail.com>



> -----邮件原件-----
> 发件人: Li,Rongqing
> 发送时间: 2020年7月21日 9:43
> 收件人: 'Magnus Karlsson' <magnus.karlsson@gmail.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>
> 主题: 答复: [Intel-wired-lan] [PATCH 1/2] xdp: i40e: ixgbe: ixgbevf: not flip rx
> buffer for copy mode xdp
> 
> 
> 
> > -----邮件原件-----
> > 发件人: 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
> 
> 

How about this?

--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -2394,7 +2394,10 @@ static int i40e_clean_rx_irq(struct i40e_ring *rx_ring, int budget)
 
                        if (xdp_res & (I40E_XDP_TX | I40E_XDP_REDIR)) {
                                xdp_xmit |= xdp_res;
-                               i40e_rx_buffer_flip(rx_ring, rx_buffer, size);
+
+                               if (xdp.rxq->mem.type == MEM_TYPE_XSK_BUFF_POOL ||
+                                   xdp_get_map_type() != BPF_MAP_TYPE_XSKMAP)
+                                       i40e_rx_buffer_flip(rx_ring, rx_buffer, size);
                        } else {
                                rx_buffer->pagecnt_bias++;
                        }
diff --git a/include/linux/filter.h b/include/linux/filter.h
index 259377723603..94f4435a77f3 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -919,6 +919,17 @@ static inline void xdp_clear_return_frame_no_direct(void)
        ri->kern_flags &= ~BPF_RI_F_RF_NO_DIRECT;
 }
 
+static enum bpf_map_type xdp_get_map_type(void)
+{
+       struct bpf_redirect_info *ri = this_cpu_ptr(&bpf_redirect_info);
+       struct bpf_map *map = READ_ONCE(ri->map);
+
+       if (map)
+               return map->map_type;
+       else
+               return BPF_MAP_TYPE_UNSPEC;
+}
+
 static inline int xdp_ok_fwd_dev(const struct net_device *fwd,
                                 unsigned int pktlen)


  parent reply	other threads:[~2020-07-21  7:49 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
2020-07-21  7:49     ` Li,Rongqing [this message]
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=b3ca06b2292742c39b7362030055869f@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).