All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] net/i40e: consider QinQ when setting MTU
@ 2017-04-28  1:55 Wenzhuo Lu
  2017-04-28  5:55 ` Ferruh Yigit
  2017-05-02  1:51 ` [PATCH v2] " Wenzhuo Lu
  0 siblings, 2 replies; 8+ messages in thread
From: Wenzhuo Lu @ 2017-04-28  1:55 UTC (permalink / raw)
  To: dev; +Cc: Wenzhuo Lu

When counting max packet length from MTU, count
VLAN tag length twice for QinQ packets.

Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c    | 2 +-
 drivers/net/i40e/i40e_ethdev_vf.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 3fa25dc..74041ae 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -10593,7 +10593,7 @@ static void i40e_set_default_mac_addr(struct rte_eth_dev *dev,
 	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
 	struct rte_eth_dev_data *dev_data = pf->dev_data;
 	uint32_t frame_size = mtu + ETHER_HDR_LEN
-			      + ETHER_CRC_LEN + I40E_VLAN_TAG_SIZE;
+			      + ETHER_CRC_LEN + I40E_VLAN_TAG_SIZE * 2;
 	int ret = 0;
 
 	/* check if mtu is within the allowed range */
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index cb34023..ac81546 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -2688,7 +2688,7 @@ static int eth_i40evf_pci_remove(struct rte_pci_device *pci_dev)
 	struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
 	struct rte_eth_dev_data *dev_data = vf->dev_data;
 	uint32_t frame_size = mtu + ETHER_HDR_LEN
-			      + ETHER_CRC_LEN + I40E_VLAN_TAG_SIZE;
+			      + ETHER_CRC_LEN + I40E_VLAN_TAG_SIZE * 2;
 	int ret = 0;
 
 	/* check if mtu is within the allowed range */
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 8+ messages in thread
* Re: [PATCH] net/i40e: consider QinQ when setting MTU
  2017-04-28  1:55 [PATCH] net/i40e: consider QinQ when setting MTU Wenzhuo Lu
@ 2017-04-28  5:55 ` Ferruh Yigit
  2017-04-28  8:27   ` Wu, Jingjing
  2017-05-02  1:51 ` [PATCH v2] " Wenzhuo Lu
  1 sibling, 1 reply; 8+ messages in thread
From: Ferruh Yigit @ 2017-04-28  5:55 UTC (permalink / raw)
  To: Wenzhuo Lu, dev

On 4/28/2017 2:55 AM, Wenzhuo Lu wrote:
> When counting max packet length from MTU, count
> VLAN tag length twice for QinQ packets.
> 
> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> ---
>  drivers/net/i40e/i40e_ethdev.c    | 2 +-
>  drivers/net/i40e/i40e_ethdev_vf.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 3fa25dc..74041ae 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -10593,7 +10593,7 @@ static void i40e_set_default_mac_addr(struct rte_eth_dev *dev,
>  	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
>  	struct rte_eth_dev_data *dev_data = pf->dev_data;
>  	uint32_t frame_size = mtu + ETHER_HDR_LEN
> -			      + ETHER_CRC_LEN + I40E_VLAN_TAG_SIZE;
> +			      + ETHER_CRC_LEN + I40E_VLAN_TAG_SIZE * 2;

Hi Wenzhuo,

Shouldn't we check if QinQ enabled before taking size account?

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH] net/i40e: consider QinQ when setting MTU
  2017-04-28  5:55 ` Ferruh Yigit
@ 2017-04-28  8:27   ` Wu, Jingjing
  2017-04-28  8:30     ` Lu, Wenzhuo
  2017-04-28 12:38     ` Ferruh Yigit
  0 siblings, 2 replies; 8+ messages in thread
From: Wu, Jingjing @ 2017-04-28  8:27 UTC (permalink / raw)
  To: Yigit, Ferruh, Lu, Wenzhuo, dev



> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ferruh Yigit
> Sent: Friday, April 28, 2017 1:55 PM
> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] net/i40e: consider QinQ when setting MTU
> 
> On 4/28/2017 2:55 AM, Wenzhuo Lu wrote:
> > When counting max packet length from MTU, count VLAN tag length twice
> > for QinQ packets.
> >
> > Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> > ---
> >  drivers/net/i40e/i40e_ethdev.c    | 2 +-
> >  drivers/net/i40e/i40e_ethdev_vf.c | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/i40e/i40e_ethdev.c
> > b/drivers/net/i40e/i40e_ethdev.c index 3fa25dc..74041ae 100644
> > --- a/drivers/net/i40e/i40e_ethdev.c
> > +++ b/drivers/net/i40e/i40e_ethdev.c
> > @@ -10593,7 +10593,7 @@ static void i40e_set_default_mac_addr(struct
> rte_eth_dev *dev,
> >  	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data-
> >dev_private);
> >  	struct rte_eth_dev_data *dev_data = pf->dev_data;
> >  	uint32_t frame_size = mtu + ETHER_HDR_LEN
> > -			      + ETHER_CRC_LEN + I40E_VLAN_TAG_SIZE;
> > +			      + ETHER_CRC_LEN + I40E_VLAN_TAG_SIZE * 2;
> 
> Hi Wenzhuo,
> 
> Shouldn't we check if QinQ enabled before taking size account?

Hi, Ferruh

Checking QinQ enabled here makes no sense, because we don't know
If the packets is carry single vlan or double vlan.

Acked-by: Jingjing Wu <jingjing.wu@intel.com>


^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH] net/i40e: consider QinQ when setting MTU
  2017-04-28  8:27   ` Wu, Jingjing
@ 2017-04-28  8:30     ` Lu, Wenzhuo
  2017-04-28 12:38     ` Ferruh Yigit
  1 sibling, 0 replies; 8+ messages in thread
From: Lu, Wenzhuo @ 2017-04-28  8:30 UTC (permalink / raw)
  To: Wu, Jingjing, Yigit, Ferruh, dev

Hi Ferruh,

> -----Original Message-----
> From: Wu, Jingjing
> Sent: Friday, April 28, 2017 4:27 PM
> To: Yigit, Ferruh; Lu, Wenzhuo; dev@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH] net/i40e: consider QinQ when setting MTU
> 
> 
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ferruh Yigit
> > Sent: Friday, April 28, 2017 1:55 PM
> > To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH] net/i40e: consider QinQ when setting
> > MTU
> >
> > On 4/28/2017 2:55 AM, Wenzhuo Lu wrote:
> > > When counting max packet length from MTU, count VLAN tag length
> > > twice for QinQ packets.
> > >
> > > Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> > > ---
> > >  drivers/net/i40e/i40e_ethdev.c    | 2 +-
> > >  drivers/net/i40e/i40e_ethdev_vf.c | 2 +-
> > >  2 files changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/net/i40e/i40e_ethdev.c
> > > b/drivers/net/i40e/i40e_ethdev.c index 3fa25dc..74041ae 100644
> > > --- a/drivers/net/i40e/i40e_ethdev.c
> > > +++ b/drivers/net/i40e/i40e_ethdev.c
> > > @@ -10593,7 +10593,7 @@ static void i40e_set_default_mac_addr(struct
> > rte_eth_dev *dev,
> > >  	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data-
> > >dev_private);
> > >  	struct rte_eth_dev_data *dev_data = pf->dev_data;
> > >  	uint32_t frame_size = mtu + ETHER_HDR_LEN
> > > -			      + ETHER_CRC_LEN + I40E_VLAN_TAG_SIZE;
> > > +			      + ETHER_CRC_LEN + I40E_VLAN_TAG_SIZE * 2;
> >
> > Hi Wenzhuo,
> >
> > Shouldn't we check if QinQ enabled before taking size account?
> 
> Hi, Ferruh
> 
> Checking QinQ enabled here makes no sense, because we don't know If the
> packets is carry single vlan or double vlan.
> 
> Acked-by: Jingjing Wu <jingjing.wu@intel.com>
BTW, the patch aligned the behavior of kernel driver and DPDK.


^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH] net/i40e: consider QinQ when setting MTU
  2017-04-28  8:27   ` Wu, Jingjing
  2017-04-28  8:30     ` Lu, Wenzhuo
@ 2017-04-28 12:38     ` Ferruh Yigit
  2017-05-02  1:25       ` Lu, Wenzhuo
  1 sibling, 1 reply; 8+ messages in thread
From: Ferruh Yigit @ 2017-04-28 12:38 UTC (permalink / raw)
  To: Wu, Jingjing, Lu, Wenzhuo, dev

On 4/28/2017 9:27 AM, Wu, Jingjing wrote:
> 
> 
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ferruh Yigit
>> Sent: Friday, April 28, 2017 1:55 PM
>> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; dev@dpdk.org
>> Subject: Re: [dpdk-dev] [PATCH] net/i40e: consider QinQ when setting MTU
>>
>> On 4/28/2017 2:55 AM, Wenzhuo Lu wrote:
>>> When counting max packet length from MTU, count VLAN tag length twice
>>> for QinQ packets.
>>>
>>> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
>>> ---
>>>  drivers/net/i40e/i40e_ethdev.c    | 2 +-
>>>  drivers/net/i40e/i40e_ethdev_vf.c | 2 +-
>>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/net/i40e/i40e_ethdev.c
>>> b/drivers/net/i40e/i40e_ethdev.c index 3fa25dc..74041ae 100644
>>> --- a/drivers/net/i40e/i40e_ethdev.c
>>> +++ b/drivers/net/i40e/i40e_ethdev.c
>>> @@ -10593,7 +10593,7 @@ static void i40e_set_default_mac_addr(struct
>> rte_eth_dev *dev,
>>>  	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data-
>>> dev_private);
>>>  	struct rte_eth_dev_data *dev_data = pf->dev_data;
>>>  	uint32_t frame_size = mtu + ETHER_HDR_LEN
>>> -			      + ETHER_CRC_LEN + I40E_VLAN_TAG_SIZE;
>>> +			      + ETHER_CRC_LEN + I40E_VLAN_TAG_SIZE * 2;
>>
>> Hi Wenzhuo,
>>
>> Shouldn't we check if QinQ enabled before taking size account?
> 
> Hi, Ferruh
> 
> Checking QinQ enabled here makes no sense, because we don't know
> If the packets is carry single vlan or double vlan.

What do you think commenting code to say "I40E_VLAN_TAG_SIZE * 2" is for
QinQ?

It is not obvious and in the feature somebody not thinking about QinQ
use case may miss this, and do something wrong, even can fix this J

> 
> Acked-by: Jingjing Wu <jingjing.wu@intel.com>
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH] net/i40e: consider QinQ when setting MTU
  2017-04-28 12:38     ` Ferruh Yigit
@ 2017-05-02  1:25       ` Lu, Wenzhuo
  0 siblings, 0 replies; 8+ messages in thread
From: Lu, Wenzhuo @ 2017-05-02  1:25 UTC (permalink / raw)
  To: Yigit, Ferruh, Wu, Jingjing, dev

Hi Ferruh,


> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Friday, April 28, 2017 8:38 PM
> To: Wu, Jingjing; Lu, Wenzhuo; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] net/i40e: consider QinQ when setting MTU
> 
> On 4/28/2017 9:27 AM, Wu, Jingjing wrote:
> >
> >
> >> -----Original Message-----
> >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ferruh Yigit
> >> Sent: Friday, April 28, 2017 1:55 PM
> >> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; dev@dpdk.org
> >> Subject: Re: [dpdk-dev] [PATCH] net/i40e: consider QinQ when setting
> >> MTU
> >>
> >> On 4/28/2017 2:55 AM, Wenzhuo Lu wrote:
> >>> When counting max packet length from MTU, count VLAN tag length
> >>> twice for QinQ packets.
> >>>
> >>> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> >>> ---
> >>>  drivers/net/i40e/i40e_ethdev.c    | 2 +-
> >>>  drivers/net/i40e/i40e_ethdev_vf.c | 2 +-
> >>>  2 files changed, 2 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/net/i40e/i40e_ethdev.c
> >>> b/drivers/net/i40e/i40e_ethdev.c index 3fa25dc..74041ae 100644
> >>> --- a/drivers/net/i40e/i40e_ethdev.c
> >>> +++ b/drivers/net/i40e/i40e_ethdev.c
> >>> @@ -10593,7 +10593,7 @@ static void
> i40e_set_default_mac_addr(struct
> >> rte_eth_dev *dev,
> >>>  	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data-
> >>> dev_private);
> >>>  	struct rte_eth_dev_data *dev_data = pf->dev_data;
> >>>  	uint32_t frame_size = mtu + ETHER_HDR_LEN
> >>> -			      + ETHER_CRC_LEN + I40E_VLAN_TAG_SIZE;
> >>> +			      + ETHER_CRC_LEN + I40E_VLAN_TAG_SIZE * 2;
> >>
> >> Hi Wenzhuo,
> >>
> >> Shouldn't we check if QinQ enabled before taking size account?
> >
> > Hi, Ferruh
> >
> > Checking QinQ enabled here makes no sense, because we don't know If
> > the packets is carry single vlan or double vlan.
> 
> What do you think commenting code to say "I40E_VLAN_TAG_SIZE * 2" is for
> QinQ?
> 
> It is not obvious and in the feature somebody not thinking about QinQ use
> case may miss this, and do something wrong, even can fix this J
Thanks for your suggestion. I'll send a V2 to add the comments.

> 
> >
> > Acked-by: Jingjing Wu <jingjing.wu@intel.com>
> >


^ permalink raw reply	[flat|nested] 8+ messages in thread
* [PATCH v2] net/i40e: consider QinQ when setting MTU
  2017-04-28  1:55 [PATCH] net/i40e: consider QinQ when setting MTU Wenzhuo Lu
  2017-04-28  5:55 ` Ferruh Yigit
@ 2017-05-02  1:51 ` Wenzhuo Lu
  2017-05-05 15:24   ` Thomas Monjalon
  1 sibling, 1 reply; 8+ messages in thread
From: Wenzhuo Lu @ 2017-05-02  1:51 UTC (permalink / raw)
  To: dev; +Cc: Wenzhuo Lu

When counting max packet length from MTU, count
VLAN tag length twice for QinQ packets.

Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
Acked-by: Jingjing Wu <jingjing.wu@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c    | 3 +--
 drivers/net/i40e/i40e_ethdev.h    | 7 +++++++
 drivers/net/i40e/i40e_ethdev_vf.c | 3 +--
 3 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 3fa25dc..e713c11 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -10592,8 +10592,7 @@ static void i40e_set_default_mac_addr(struct rte_eth_dev *dev,
 {
 	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
 	struct rte_eth_dev_data *dev_data = pf->dev_data;
-	uint32_t frame_size = mtu + ETHER_HDR_LEN
-			      + ETHER_CRC_LEN + I40E_VLAN_TAG_SIZE;
+	uint32_t frame_size = mtu + I40E_ETH_OVERHEAD;
 	int ret = 0;
 
 	/* check if mtu is within the allowed range */
diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
index 71b0874..2ff8282 100644
--- a/drivers/net/i40e/i40e_ethdev.h
+++ b/drivers/net/i40e/i40e_ethdev.h
@@ -251,6 +251,13 @@ enum i40e_flxpld_layer_idx {
 	I40E_INSET_FLEX_PAYLOAD_W5 | I40E_INSET_FLEX_PAYLOAD_W6 | \
 	I40E_INSET_FLEX_PAYLOAD_W7 | I40E_INSET_FLEX_PAYLOAD_W8)
 
+/**
+ * The overhead from MTU to max frame size.
+ * Considering QinQ packet, the VLAN tag needs to be counted twice.
+ */
+#define I40E_ETH_OVERHEAD \
+	(ETHER_HDR_LEN + ETHER_CRC_LEN + I40E_VLAN_TAG_SIZE * 2)
+
 struct i40e_adapter;
 
 /**
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index cb34023..d27f09d 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -2687,8 +2687,7 @@ static int eth_i40evf_pci_remove(struct rte_pci_device *pci_dev)
 {
 	struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
 	struct rte_eth_dev_data *dev_data = vf->dev_data;
-	uint32_t frame_size = mtu + ETHER_HDR_LEN
-			      + ETHER_CRC_LEN + I40E_VLAN_TAG_SIZE;
+	uint32_t frame_size = mtu + I40E_ETH_OVERHEAD;
 	int ret = 0;
 
 	/* check if mtu is within the allowed range */
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 8+ messages in thread
* Re: [PATCH v2] net/i40e: consider QinQ when setting MTU
  2017-05-02  1:51 ` [PATCH v2] " Wenzhuo Lu
@ 2017-05-05 15:24   ` Thomas Monjalon
  0 siblings, 0 replies; 8+ messages in thread
From: Thomas Monjalon @ 2017-05-05 15:24 UTC (permalink / raw)
  To: Wenzhuo Lu; +Cc: dev

02/05/2017 03:51, Wenzhuo Lu:
> When counting max packet length from MTU, count
> VLAN tag length twice for QinQ packets.
> 
> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> Acked-by: Jingjing Wu <jingjing.wu@intel.com>

Applied, thanks

^ permalink raw reply	[flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-05-05 15:24 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-28  1:55 [PATCH] net/i40e: consider QinQ when setting MTU Wenzhuo Lu
2017-04-28  5:55 ` Ferruh Yigit
2017-04-28  8:27   ` Wu, Jingjing
2017-04-28  8:30     ` Lu, Wenzhuo
2017-04-28 12:38     ` Ferruh Yigit
2017-05-02  1:25       ` Lu, Wenzhuo
2017-05-02  1:51 ` [PATCH v2] " Wenzhuo Lu
2017-05-05 15:24   ` Thomas Monjalon

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.