Re: [dpdk-dev] [PATCH v2 6/7] net/e1000: set min and max MTU for igb devices

["Zhang, Qi Z" <qi.z.zhang@intel.com>]

> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Monday, March 25, 2019 10:25 PM
> To: Stokes, Ian <ian.stokes@intel.com>; dev@dpdk.org
> Cc: stephen@networkplumber.org; Lu, Wenzhuo <wenzhuo.lu@intel.com>;
> Ananyev, Konstantin <konstantin.ananyev@intel.com>; Xing, Beilei
> <beilei.xing@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v2 6/7] net/e1000: set min and max MTU for igb
> devices
> 
> On 3/22/2019 1:01 PM, Ian Stokes wrote:
> > This commit sets the min and max supported MTU values for igb devices
> > via the eth_igb_info_get() function. Min MTU supported is set to
> > ETHER_MIN_MTU and max mtu is calculated as the max packet length
> > supported minus the transport overhead. To aid in these calculations a
> > new MACRO 'E1000_ETH_OVERHEAD' has been introduced to consolidate
> > overhead calculation and avoid duplication.
> >
> > Signed-off-by: Ian Stokes <ian.stokes@intel.com>
> > ---
> >  drivers/net/e1000/e1000_ethdev.h | 6 ++++++
> >  drivers/net/e1000/igb_ethdev.c   | 7 +++++--
> >  2 files changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/e1000/e1000_ethdev.h
> > b/drivers/net/e1000/e1000_ethdev.h
> > index 94edff08e..3e74cd8fe 100644
> > --- a/drivers/net/e1000/e1000_ethdev.h
> > +++ b/drivers/net/e1000/e1000_ethdev.h
> > @@ -89,6 +89,12 @@
> >  	ETH_RSS_IPV6_UDP_EX)
> >
> >  /*
> > + * The overhead from MTU to max frame size.
> > + * Considering VLAN so a tag needs to be counted.
> > + */
> > +#define E1000_ETH_OVERHEAD (ETHER_HDR_LEN + ETHER_CRC_LEN +
> > +VLAN_TAG_SIZE)
> 
> As an overhead, following drivers set:
> i40e: HDR + CRC + 2 * VLAN
> ixgbe: HDR + CRC
> e1000: HDR + CRC + VLAN
> 
> I wonder if this difference is HW limitation, or driver limitation or just
> implementation inconsistency.

I think this is implementation inconsistency 

The NIC only accept Max Frame Size.

The problem here is seems all of three setup are not perfect.

HDR + CRC + 2 * VLAN - it may allow non vlan or single vlan packet that exceed mtu. 
HDR + CRC - it may reject vlan or double vlan packet that follow mtu.  
HDR + CRC + VLAN ,  it may reject double vlan packet that follow mtu

I agree it's better to keep consistent on all drivers, but before this, we may need to decide which one we should take :)

Regards
Qi

> 
> Better to confirm it that it is not implementation inconsistency.
> 
> Wenzhuo, Konstantin, Beilei, Qi,
> 
> Can you please comment?
> 
> Thanks,
> ferruh