Date:   Wed, 8 Feb 2023 20:02:03 +0100
From:   Maciej Fijalkowski <maciej.fijalkowski@intel.com>
To:     Alexander H Duyck <alexander.duyck@gmail.com>
CC:     Jason Xing <kerneljasonxing@gmail.com>,
        <jesse.brandeburg@intel.com>, <anthony.l.nguyen@intel.com>,
        <davem@davemloft.net>, <edumazet@google.com>, <kuba@kernel.org>,
        <pabeni@redhat.com>, <richardcochran@gmail.com>, <ast@kernel.org>,
        <daniel@iogearbox.net>, <hawk@kernel.org>,
        <john.fastabend@gmail.com>, <alexandr.lobakin@intel.com>,
        <intel-wired-lan@lists.osuosl.org>, <netdev@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <bpf@vger.kernel.org>,
        Jason Xing <kernelxing@tencent.com>
Subject: Re: [PATCH net v4 1/3] ixgbe: allow to increase MTU to 3K with XDP
 enabled
Message-ID: <Y+Pxq10w/4e1HT+4@boxer>
References: <20230208024333.10465-1-kerneljasonxing@gmail.com>
 <2bfcd7d92a6971416f58d9aac6e74840d5ae240a.camel@gmail.com>
 <Y+PNjcrSxKc0vD3s@boxer>
 <6ded592b01ba223bce241d6ff3073246cb5dd18b.camel@gmail.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <6ded592b01ba223bce241d6ff3073246cb5dd18b.camel@gmail.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?nWv2N9eelMXHx5iblN7Vpd2m8QRUZLu5i2eGpjCfOvX9znQaPv/VPO43iGfA?=
 =?us-ascii?Q?0ljO0MVANZLUA78ixTJmUgc5BoIhZjRNDu9LGzGgnkxH+LwG63FmMpckxdaU?=
 =?us-ascii?Q?DOPdjcykLyx5z9DtmN5ngcDsqsPBVz8bmrEJQGI1l3wcM3rJhwy2WHssAr5L?=
 =?us-ascii?Q?OI7jTqMQBmJk5XR0FOyboWfKhbctR5wXyoy0+ykIJu9VoEloeVYmkObCUYh1?=
 =?us-ascii?Q?r9rscDM9sIs3UJgrMpfSFafkWo4wu7JAEhmY/lwHyISbnY8aeQd6cVIZ/iea?=
 =?us-ascii?Q?0U6GFY1R8nlhjz44AWLYl+5NVysO/dHSkxco9JEUUtIvRs+of2HKOTfksS6J?=
 =?us-ascii?Q?PjjEwQenNbhBoY1nwjwyUs6QOh2PFR8H3NsqDeEjYYeDdH3GmzRMdWV+P8bC?=
 =?us-ascii?Q?+zXguf0kHIpQ1lTFXUSy0RI9LNnyEvrWsbPRHGp5kGQcBohMoSY28bgwDgfs?=
 =?us-ascii?Q?O/SOhbwmDusL4JVt/F0LabCl0xY5/0xHGW/e0cTfwlaiv9Py0sebjbIOBZbo?=
 =?us-ascii?Q?9GwTKOG6yH3z/DcLFzbyiCsGc5s+lv/a1tnGEMAwS1RO9m2lV6oSvwJpPXVs?=
 =?us-ascii?Q?QoViBZlrvuZQEsLnMOECd4pr7MndP0XQsujSoo4H9DjofGDFShyirfI8zYzv?=
 =?us-ascii?Q?+0OZY8PTRPZVNsniWN2FdIhUTzUPiHXjyuMMcUnZoND82eaMjELtd3FVQpMh?=
 =?us-ascii?Q?Ks2T+w+/m2XgeVlijgMPVlpYA9Z3yedQi9lHDR8kWJOwkTh4znV0HpM65LYy?=
 =?us-ascii?Q?MeZO8ZIix8khxFFV/87WsyhxyPKcfJJKi+LEtvVVBmUIuYdDPga3UKkTZH55?=
 =?us-ascii?Q?1XfdwCYgwbqgrPaEnC9z+xNLhaYjK/opbIGF4Lv/MYPi6b65P3EjAAinL7nN?=
 =?us-ascii?Q?orfWA+drelw4Ckl0UbQpQXDyeRFV9XLu5mJ5+uljsCYHx4oPjYn5zp15dX6+?=
 =?us-ascii?Q?OxnV1JOyibWClC6tuMQsfUjBO9RzNcQzYlSDSoiMUql1u0L21wLGZCyGamm4?=
 =?us-ascii?Q?0QiUDAsoRGlIr05Etf01vQoHS6GR1sk2wW4hesE8dzf6BLgOfmcfgNk+Rsh0?=
 =?us-ascii?Q?imSYVCujbNG+msg7K0wzWrtdLxgsppJjSJhU95sqk2FaM5X3n62QQ3Tyz5A3?=
 =?us-ascii?Q?D3zclYtdlBs5wjHQ5GjCdHEQBpqa7EoEVfJL8kZIJAwgQC1LZvEun1b1YNgP?=
 =?us-ascii?Q?XUVpSa9gvLC+23M5OVRCESEnQNblIzkQSlZY8gNGHELz3efZfCzzIpZa8kv2?=
 =?us-ascii?Q?QVHHko07WStad07e1/w/E3rsCRUw3Uw0gZHl4SRG8qBVwqRbxSvoKEM/oZaA?=
 =?us-ascii?Q?SmnVhnYkC/vgYzK1XA51S0DNz2bKFw2l6m2IUR5m2yLttvN0aEj7WcJiyU4K?=
 =?us-ascii?Q?5PAYF93JXz5+1lBYMfemf5wgNH1V+NE+k/Lx8jwp2lDNymWXEwP6bnd4lUoi?=
 =?us-ascii?Q?oKpg0y1Asw9A7bCvkvPTcPxg9WnaFqmVIkFxfYxEZyaAo4sf8xtsFrm+Yg8f?=
 =?us-ascii?Q?iKTroOrXvU+GrHVAsXEZ9rPANerEeBCJ9mybP57A1b/rWxaec6RfhJsOuE1Z?=
 =?us-ascii?Q?scjbYT7bxH0kSiLBTaEOl/0pQC+Fq/JO7CHtiEoC2VQDz2Q6UcTwjYrGI9g9?=
 =?us-ascii?Q?i6AEanSJbIArZ9Gy6TqRDwA=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 115de4f5-e89a-4623-099d-08db0a06fbdb
X-MS-Exchange-CrossTenant-AuthSource: DM4PR11MB6117.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Feb 2023 19:02:12.0198
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: Bez7fzn4pbM+zKg7iR6rnHiyerWpWcZIJbPBKM7zAHZhh5RYEZS7BwrdjbRLv7+U7FP0/8DhqT2JPqLc63Jz9wo7dOT9/S7XIaA74kZfbE8=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA1PR11MB6687
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Wed, Feb 08, 2023 at 10:57:22AM -0800, Alexander H Duyck wrote:
> On Wed, 2023-02-08 at 17:27 +0100, Maciej Fijalkowski wrote:
> > On Wed, Feb 08, 2023 at 07:37:57AM -0800, Alexander H Duyck wrote:
> > > On Wed, 2023-02-08 at 10:43 +0800, Jason Xing wrote:
> > > > From: Jason Xing <kernelxing@tencent.com>
> > > > 
> > > > Recently I encountered one case where I cannot increase the MTU size
> > > > directly from 1500 to a much bigger value with XDP enabled if the
> > > > server is equipped with IXGBE card, which happened on thousands of
> > > > servers in production environment. After appling the current patch,
> > > > we can set the maximum MTU size to 3K.
> > > > 
> > > > This patch follows the behavior of changing MTU as i40e/ice does.
> > > > 
> > > > Referrences:
> > > > [1] commit 23b44513c3e6 ("ice: allow 3k MTU for XDP")
> > > > [2] commit 0c8493d90b6b ("i40e: add XDP support for pass and drop actions")
> > > > 
> > > > Fixes: fabf1bce103a ("ixgbe: Prevent unsupported configurations with XDP")
> > > > Signed-off-by: Jason Xing <kernelxing@tencent.com>
> > > 
> > > This is based on the broken premise that w/ XDP we are using a 4K page.
> > > The ixgbe driver isn't using page pool and is therefore running on
> > > different limitations. The ixgbe driver is only using 2K slices of the
> > > 4K page. In addition that is reduced to 1.5K to allow for headroom and
> > > the shared info in the buffer.
> > > 
> > > Currently the only way a 3K buffer would work is if FCoE is enabled and
> > > in that case the driver is using order 1 pages and still using the
> > > split buffer approach.
> > 
> > Hey Alex, interesting, we based this on the following logic from
> > ixgbe_set_rx_buffer_len() I guess:
> > 
> > #if (PAGE_SIZE < 8192)
> > 		if (adapter->flags2 & IXGBE_FLAG2_RSC_ENABLED)
> > 			set_bit(__IXGBE_RX_3K_BUFFER, &rx_ring->state);
> > 
> > 		if (IXGBE_2K_TOO_SMALL_WITH_PADDING ||
> > 		    (max_frame > (ETH_FRAME_LEN + ETH_FCS_LEN)))
> > 			set_bit(__IXGBE_RX_3K_BUFFER, &rx_ring->state);
> > #endif
> > 
> > so we assumed that ixgbe is no different than i40e/ice in these terms, but
> > we ignored whole overhead of LRO/RSC that ixgbe carries.
> 
> If XDP is already enabled the LRO/RSC cannot be enabled. I think that
> is already disabled if we have XDP enabled.
> 
> > I am not actively working with ixgbe but I know that you were the main dev
> > of it, so without premature dive into the datasheet and codebase, are you
> > really sure that 3k mtu for XDP is a no go?
> 
> I think I mixed up fm10k and ixgbe, either that or I was thinking of
> the legacy setup. They all kind of blur together as I had worked on
> pretty much all the Intel drivers up to i40e the last time I was
> updating them for all the Rx path stuff. :)
> 
> So if I am reading things right the issue is that if XDP is enabled you
> cannot set a 3K MTU, but if you set the 3K MTU first then you can
> enable XDP after the fact right?

Yes and vice versa - when XDP is on then you should be able to work with
3k mtus.

> 
> Looking it over again after re-reading the code this looks good to me.
> 
> Reviewed-by: Alexander Duyck <alexanderduyck@fb.com>

Awesome :)

> 
> 
> 
> 
>