Message ID | 20170303022448.79638-3-jeffrey.t.kirsher@intel.com |
---|---|
State | Accepted, archived |
Delegated to: | David Miller |
Headers | show |
From: Jeff Kirsher > Sent: 03 March 2017 02:25 > From: Alexander Duyck <alexander.h.duyck@intel.com> > > On architectures that have a cache line size larger than 64 Bytes we start > running into issues where the amount of headroom for the frame starts > shrinking. > > The size of skb_shared_info on a system with a 64B L1 cache line size is > 320. This increases to 384 with a 128B cache line, and 512 with a 256B > cache line. Perhaps some of the CACHE_LINE_ALIGNED markers don't actually need to force alignment with large line sizes? I realise some things have hard requirements for cache alignment (eg non-coherent dma), but others are just there to limit the number of cache lines read and/or dirtied. David
> -----Original Message----- > From: David Laight [mailto:David.Laight@ACULAB.COM] > Sent: Friday, March 3, 2017 4:25 AM > To: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>; davem@davemloft.net > Cc: Duyck, Alexander H <alexander.h.duyck@intel.com>; > netdev@vger.kernel.org; nhorman@redhat.com; sassmann@redhat.com; > jogreene@redhat.com > Subject: RE: [net 2/2] ixgbe: Limit use of 2K buffers on architectures with 256B > or larger cache lines > > From: Jeff Kirsher > > Sent: 03 March 2017 02:25 > > From: Alexander Duyck <alexander.h.duyck@intel.com> > > > > On architectures that have a cache line size larger than 64 Bytes we > > start running into issues where the amount of headroom for the frame > > starts shrinking. > > > > The size of skb_shared_info on a system with a 64B L1 cache line size > > is 320. This increases to 384 with a 128B cache line, and 512 with a > > 256B cache line. > > Perhaps some of the CACHE_LINE_ALIGNED markers don't actually need to > force alignment with large line sizes? > > I realise some things have hard requirements for cache alignment (eg non- > coherent dma), but others are just there to limit the number of cache lines read > and/or dirtied. > > David For our purposes I think this works well enough. Basically we wanted to guarantee we have enough headroom for XDP. In the case of the Mellanox drivers they are guaranteeing 256 if I recall correctly. I have some follow-up patches for net-next that will make it so that we can just do a build-time test that will determine the padding size and allow us to always guaranteed at least NET_SKB_PAD + NET_IP_ALIGN. - Alex
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h index 7a951b116821..b1ecc2627a5a 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h @@ -96,7 +96,7 @@ #define IXGBE_MAX_FRAME_BUILD_SKB \ (SKB_WITH_OVERHEAD(IXGBE_RXBUFFER_2K) - IXGBE_SKB_PAD) #else -#define IGB_MAX_FRAME_BUILD_SKB IXGBE_RXBUFFER_2K +#define IXGBE_MAX_FRAME_BUILD_SKB IXGBE_RXBUFFER_2K #endif /* diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c index 67ab13fd163c..a7a430a7be2c 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c @@ -3972,7 +3972,8 @@ static void ixgbe_set_rx_buffer_len(struct ixgbe_adapter *adapter) if (adapter->flags2 & IXGBE_FLAG2_RSC_ENABLED) set_bit(__IXGBE_RX_3K_BUFFER, &rx_ring->state); - if (max_frame > (ETH_FRAME_LEN + ETH_FCS_LEN)) + if ((max_frame > (ETH_FRAME_LEN + ETH_FCS_LEN)) || + (max_frame > IXGBE_MAX_FRAME_BUILD_SKB)) set_bit(__IXGBE_RX_3K_BUFFER, &rx_ring->state); #endif }