From patchwork Tue Jul 21 07:49:00 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Li RongQing X-Patchwork-Id: 1332832 X-Patchwork-Delegate: bpf@iogearbox.net Return-Path: X-Original-To: patchwork-incoming-netdev@ozlabs.org Delivered-To: patchwork-incoming-netdev@ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org (client-ip=23.128.96.18; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=baidu.com Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by ozlabs.org (Postfix) with ESMTP id 4B9rMc5KJLz9sRR for ; Tue, 21 Jul 2020 17:49:16 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727849AbgGUHtO (ORCPT ); Tue, 21 Jul 2020 03:49:14 -0400 Received: from mx24.baidu.com ([111.206.215.185]:41266 "EHLO baidu.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1725984AbgGUHtO (ORCPT ); Tue, 21 Jul 2020 03:49:14 -0400 Received: from BJHW-Mail-Ex13.internal.baidu.com (unknown [10.127.64.36]) by Forcepoint Email with ESMTPS id 67670E92BED48DA19C78; Tue, 21 Jul 2020 15:49:06 +0800 (CST) Received: from BJHW-Mail-Ex13.internal.baidu.com (10.127.64.36) by BJHW-Mail-Ex13.internal.baidu.com (10.127.64.36) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1979.3; Tue, 21 Jul 2020 15:49:06 +0800 Received: from BJHW-Mail-Ex13.internal.baidu.com ([100.100.100.36]) by BJHW-Mail-Ex13.internal.baidu.com ([100.100.100.36]) with mapi id 15.01.1979.003; Tue, 21 Jul 2020 15:49:00 +0800 From: "Li,Rongqing" To: Magnus Karlsson CC: Network Development , intel-wired-lan , "Karlsson, Magnus" , =?utf-8?b?QmrDtnJuIFTDtnBlbA==?= Subject: =?utf-8?q?=E7=AD=94=E5=A4=8D=3A_=5BIntel-wired-lan=5D_=5BPATCH_1/2?= =?utf-8?q?=5D_xdp=3A_i40e=3A_ixgbe=3A_ixgbevf=3A_not_flip_rx_buffer_for_cop?= =?utf-8?q?y_mode_xdp?= Thread-Topic: [Intel-wired-lan] [PATCH 1/2] xdp: i40e: ixgbe: ixgbevf: not flip rx buffer for copy mode xdp Thread-Index: AQHWXmZtsIr9AHsudEOLFGU9se5TiKkRPZUQgABrUQA= Date: Tue, 21 Jul 2020 07:49:00 +0000 Message-ID: References: <1594967062-20674-1-git-send-email-lirongqing@baidu.com> <1594967062-20674-2-git-send-email-lirongqing@baidu.com> Accept-Language: zh-CN, en-US Content-Language: zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [172.22.197.254] x-baidu-bdmsfe-datecheck: 1_BJHW-Mail-Ex13_2020-07-21 15:49:06:416 MIME-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org > -----邮件原件----- > 发件人: Li,Rongqing > 发送时间: 2020年7月21日 9:43 > 收件人: 'Magnus Karlsson' > 抄送: Network Development ; intel-wired-lan > ; Karlsson, Magnus > ; Björn Töpel > 主题: 答复: [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 > > 抄送: Network Development ; intel-wired-lan > > ; Karlsson, Magnus > > ; Björn Töpel > > 主题: 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 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)