* [PATCH v6] net/virtio: add set_mtu in virtio
@ 2016-09-21 23:11 Dey
2016-09-21 23:22 ` Stephen Hemminger
0 siblings, 1 reply; 17+ messages in thread
From: Dey @ 2016-09-21 23:11 UTC (permalink / raw)
To: mark.b.kavanagh, yuanhan.liu, stephen, dev; +Cc: souvikdey33
From: souvikdey33 <sodey@sonusnet.com>
Virtio interfaces do not currently allow the user to specify a particular
Maximum Transmission Unit (MTU).Consequently, the MTU of Virtio interfaces
is typically set to the Ethernet default value of 1500.
This is problematic in the case of cloud deployments, in which a specific
(and potentially non-standard) MTU needs to be set by a DHCP server, which
needs to be honored by all interfaces across the traffic path.To acheive
this Virtio interfaces should support setting of MTU.
In case when GRE/VXLAN tunneling is used for internal communication, there
will be an overhead added by the infrastructure in the packet over and
above the ETHER MTU of 1518. So to take care of this overhead in these
cases the DHCP server corrects the L3 MTU to 1454. But since virtio
interfaces was not having the MTU set functionality that MTU sent by the
DHCP server was ignored and the instance will still send packets with 1500
MTU which after encapsulation will become more than 1518 and eventually
gets dropped in the infrastructure.
By adding an additional 'set_mtu' function to the Virtio driver, we can
honor the MTU sent by the DHCP server. The dhcp server/controller can
then leverage this 'set_mtu' functionality to resolve the above
mentioned issue of packets getting dropped due to incorrect size.
Signed-off-by: Souvik Dey <sodey@sonusnet.com>
---
Changes in v6:
- Description of change corrected
- Corrected the identations
- Corrected the subject line too
- The From line was also not correct
Changes in v5:
- Fix log message for out-of-bounds MTU parameter in virtio_mtu_set
- Calculate frame size, based on 'mtu' parameter
- Corrected the upper bound and lower bound checks in virtio_mtu_set
Changes in v4: Incorporated review comments.
Changes in v3: Corrected few style errors as reported by sys-stv.
Changes in v2: Incorporated review comments.
drivers/net/virtio/virtio_ethdev.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 423c597..1dbfea6 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
+
+#define VLAN_TAG_SIZE 4 /* 802.3ac tag (not DMA'd) */
+
+static int virtio_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
+{
+ struct rte_eth_dev_info dev_info;
+ uint32_t ether_hdr_len = ETHER_HDR_LEN + ETHER_CRC_LEN + VLAN_TAG_SIZE;
+ uint32_t frame_size = mtu + ether_hdr_len;
+
+ virtio_dev_info_get(dev, &dev_info);
+
+ if (mtu < ETHER_MIN_MTU || frame_size > dev_info.max_rx_pktlen) {
+ PMD_INIT_LOG(ERR, "MTU should be between %d and %d\n",
+ ETHER_MIN_MTU,
+ (dev_info.max_rx_pktlen - ether_hdr_len));
+ return -EINVAL;
+ }
+ return 0;
+}
/*
* dev_ops for virtio, bare necessities for basic operation
*/
@@ -677,7 +685,6 @@ static const struct eth_dev_ops virtio_eth_dev_ops = {
.allmulticast_enable = virtio_dev_allmulticast_enable,
.allmulticast_disable = virtio_dev_allmulticast_disable,
+ .mtu_set = virtio_mtu_set,
.dev_infos_get = virtio_dev_info_get,
.stats_get = virtio_dev_stats_get,
.xstats_get = virtio_dev_xstats_get,
--
2.9.3.windows.1
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v6] net/virtio: add set_mtu in virtio
2016-09-21 23:11 [PATCH v6] net/virtio: add set_mtu in virtio Dey
@ 2016-09-21 23:22 ` Stephen Hemminger
2016-09-22 0:08 ` Dey, Souvik
0 siblings, 1 reply; 17+ messages in thread
From: Stephen Hemminger @ 2016-09-21 23:22 UTC (permalink / raw)
To: Dey; +Cc: mark.b.kavanagh, yuanhan.liu, dev
On Wed, 21 Sep 2016 19:11:47 -0400
Dey <sodey@sonusnet.com> wrote:
>
> +
> +#define VLAN_TAG_SIZE 4 /* 802.3ac tag (not DMA'd) */
> +
> +static int virtio_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
> +{
> + struct rte_eth_dev_info dev_info;
> + uint32_t ether_hdr_len = ETHER_HDR_LEN + ETHER_CRC_LEN + VLAN_TAG_SIZE;
> + uint32_t frame_size = mtu + ether_hdr_len;
> +
> + virtio_dev_info_get(dev, &dev_info);
> +
> + if (mtu < ETHER_MIN_MTU || frame_size > dev_info.max_rx_pktlen) {
> + PMD_INIT_LOG(ERR, "MTU should be between %d and %d\n",
> + ETHER_MIN_MTU,
> + (dev_info.max_rx_pktlen - ether_hdr_len));
> + return -EINVAL;
> + }
> + return 0;
> +}
I am fine with the general idea of this patch but:
1. Calling virtio_dev_info_get is needlessly wasteful when all you want
is to access the max packet length. Since max_rx_pktlen is always
VIRTIO_MAX_RX_PKTLEN, please just use that.
2. Defining VLAN_TAG_SIZE is irrelevant if doing vlan offload.
3. Virtio doesn't insert CRC, therefore CRC_LEN is irrelevant
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v6] net/virtio: add set_mtu in virtio
2016-09-21 23:22 ` Stephen Hemminger
@ 2016-09-22 0:08 ` Dey, Souvik
2016-09-22 1:45 ` Stephen Hemminger
2016-09-22 15:57 ` Stephen Hemminger
0 siblings, 2 replies; 17+ messages in thread
From: Dey, Souvik @ 2016-09-22 0:08 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: mark.b.kavanagh, yuanhan.liu, dev
Answers inline.
--
Regards,
Souvik
-----Original Message-----
From: Stephen Hemminger [mailto:stephen@networkplumber.org]
Sent: Wednesday, September 21, 2016 7:22 PM
To: Dey, Souvik <sodey@sonusnet.com>
Cc: mark.b.kavanagh@intel.com; yuanhan.liu@linux.intel.com; dev@dpdk.org
Subject: Re: [PATCH v6] net/virtio: add set_mtu in virtio
On Wed, 21 Sep 2016 19:11:47 -0400
Dey <sodey@sonusnet.com> wrote:
>
> +
> +#define VLAN_TAG_SIZE 4 /* 802.3ac tag (not DMA'd) */
> +
> +static int virtio_mtu_set(struct rte_eth_dev *dev, uint16_t mtu) {
> + struct rte_eth_dev_info dev_info;
> + uint32_t ether_hdr_len = ETHER_HDR_LEN + ETHER_CRC_LEN + VLAN_TAG_SIZE;
> + uint32_t frame_size = mtu + ether_hdr_len;
> +
> + virtio_dev_info_get(dev, &dev_info);
> +
> + if (mtu < ETHER_MIN_MTU || frame_size > dev_info.max_rx_pktlen) {
> + PMD_INIT_LOG(ERR, "MTU should be between %d and %d\n",
> + ETHER_MIN_MTU,
> + (dev_info.max_rx_pktlen - ether_hdr_len));
> + return -EINVAL;
> + }
> + return 0;
> +}
I am fine with the general idea of this patch but:
1. Calling virtio_dev_info_get is needlessly wasteful when all you want
is to access the max packet length. Since max_rx_pktlen is always
VIRTIO_MAX_RX_PKTLEN, please just use that.
[Dey, Souvik] I am using the virtio_dev_info_get as in future can/may support the max_rx_pktlen as a variable to be set by the application. This will keep the changes future proof. As we need to support till 65535 instead of 9728 as the linux does.
2. Defining VLAN_TAG_SIZE is irrelevant if doing vlan offload.
[Dey, Souvik] vlan offload is not mandatory. Se again still have vlan being sent up to the application. In that case we need to consider the vlan length in the Ethernet size.
3. Virtio doesn't insert CRC, therefore CRC_LEN is irrelevant
[Dey, Souvik] I am not sure of this. Mark commented earlier to consider this length too. Mark what do you suggest ?
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v6] net/virtio: add set_mtu in virtio
2016-09-22 0:08 ` Dey, Souvik
@ 2016-09-22 1:45 ` Stephen Hemminger
2016-09-22 2:29 ` Dey, Souvik
2016-09-23 7:53 ` Yuanhan Liu
2016-09-22 15:57 ` Stephen Hemminger
1 sibling, 2 replies; 17+ messages in thread
From: Stephen Hemminger @ 2016-09-22 1:45 UTC (permalink / raw)
To: Dey, Souvik; +Cc: mark.b.kavanagh, yuanhan.liu, dev
On Thu, 22 Sep 2016 00:08:38 +0000
"Dey, Souvik" <sodey@sonusnet.com> wrote:
> Answers inline.
>
> --
> Regards,
> Souvik
>
> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Wednesday, September 21, 2016 7:22 PM
> To: Dey, Souvik <sodey@sonusnet.com>
> Cc: mark.b.kavanagh@intel.com; yuanhan.liu@linux.intel.com; dev@dpdk.org
> Subject: Re: [PATCH v6] net/virtio: add set_mtu in virtio
>
> On Wed, 21 Sep 2016 19:11:47 -0400
> Dey <sodey@sonusnet.com> wrote:
>
> >
> > +
> > +#define VLAN_TAG_SIZE 4 /* 802.3ac tag (not DMA'd) */
> > +
> > +static int virtio_mtu_set(struct rte_eth_dev *dev, uint16_t mtu) {
> > + struct rte_eth_dev_info dev_info;
> > + uint32_t ether_hdr_len = ETHER_HDR_LEN + ETHER_CRC_LEN + VLAN_TAG_SIZE;
> > + uint32_t frame_size = mtu + ether_hdr_len;
> > +
> > + virtio_dev_info_get(dev, &dev_info);
> > +
> > + if (mtu < ETHER_MIN_MTU || frame_size > dev_info.max_rx_pktlen) {
> > + PMD_INIT_LOG(ERR, "MTU should be between %d and %d\n",
> > + ETHER_MIN_MTU,
> > + (dev_info.max_rx_pktlen - ether_hdr_len));
> > + return -EINVAL;
> > + }
> > + return 0;
> > +}
>
> I am fine with the general idea of this patch but:
> 1. Calling virtio_dev_info_get is needlessly wasteful when all you want
> is to access the max packet length. Since max_rx_pktlen is always
> VIRTIO_MAX_RX_PKTLEN, please just use that.
> [Dey, Souvik] I am using the virtio_dev_info_get as in future can/may support the max_rx_pktlen as a variable to be set by the application. This will keep the changes future proof. As we need to support till 65535 instead of 9728 as the linux does.
Fine, then just dereference hw->rx_max_pktlen. Driver code can/should reference
its own data directly.
>
> 2. Defining VLAN_TAG_SIZE is irrelevant if doing vlan offload.
> [Dey, Souvik] vlan offload is not mandatory. Se again still have vlan being sent up to the application. In that case we need to consider the vlan length in the Ethernet size.
The code needs to handle both vlan offload (or not), correctly. You are assuming
the worst case here.
>
> 3. Virtio doesn't insert CRC, therefore CRC_LEN is irrelevant
> [Dey, Souvik] I am not sure of this. Mark commented earlier to consider this length too. Mark what do you suggest ?
Actually, the thing that matters is the size of the merge rx buf header, not the CRC.
The patch is still buggy.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v6] net/virtio: add set_mtu in virtio
2016-09-22 1:45 ` Stephen Hemminger
@ 2016-09-22 2:29 ` Dey, Souvik
2016-09-23 7:53 ` Yuanhan Liu
1 sibling, 0 replies; 17+ messages in thread
From: Dey, Souvik @ 2016-09-22 2:29 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: mark.b.kavanagh, yuanhan.liu, dev
I still fail to understand how the dev_info.max_rx_pktlen is wrong here, and the hw->rx_max_pktlen is defined at all ? If you see the other driver code we have used the same way there too. Also for the vlan part isn't it better the take the worst case for the mtu lower boundary check, as that will be valid for both vlan offload on and off cases.
-----Original Message-----
From: Stephen Hemminger [mailto:stephen@networkplumber.org]
Sent: Wednesday, September 21, 2016 9:45 PM
To: Dey, Souvik <sodey@sonusnet.com>
Cc: mark.b.kavanagh@intel.com; yuanhan.liu@linux.intel.com; dev@dpdk.org
Subject: Re: [PATCH v6] net/virtio: add set_mtu in virtio
On Thu, 22 Sep 2016 00:08:38 +0000
"Dey, Souvik" <sodey@sonusnet.com> wrote:
> Answers inline.
>
> --
> Regards,
> Souvik
>
> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Wednesday, September 21, 2016 7:22 PM
> To: Dey, Souvik <sodey@sonusnet.com>
> Cc: mark.b.kavanagh@intel.com; yuanhan.liu@linux.intel.com;
> dev@dpdk.org
> Subject: Re: [PATCH v6] net/virtio: add set_mtu in virtio
>
> On Wed, 21 Sep 2016 19:11:47 -0400
> Dey <sodey@sonusnet.com> wrote:
>
> >
> > +
> > +#define VLAN_TAG_SIZE 4 /* 802.3ac tag (not DMA'd) */
> > +
> > +static int virtio_mtu_set(struct rte_eth_dev *dev, uint16_t mtu) {
> > + struct rte_eth_dev_info dev_info;
> > + uint32_t ether_hdr_len = ETHER_HDR_LEN + ETHER_CRC_LEN + VLAN_TAG_SIZE;
> > + uint32_t frame_size = mtu + ether_hdr_len;
> > +
> > + virtio_dev_info_get(dev, &dev_info);
> > +
> > + if (mtu < ETHER_MIN_MTU || frame_size > dev_info.max_rx_pktlen) {
> > + PMD_INIT_LOG(ERR, "MTU should be between %d and %d\n",
> > + ETHER_MIN_MTU,
> > + (dev_info.max_rx_pktlen - ether_hdr_len));
> > + return -EINVAL;
> > + }
> > + return 0;
> > +}
>
> I am fine with the general idea of this patch but:
> 1. Calling virtio_dev_info_get is needlessly wasteful when all you want
> is to access the max packet length. Since max_rx_pktlen is always
> VIRTIO_MAX_RX_PKTLEN, please just use that.
> [Dey, Souvik] I am using the virtio_dev_info_get as in future can/may support the max_rx_pktlen as a variable to be set by the application. This will keep the changes future proof. As we need to support till 65535 instead of 9728 as the linux does.
Fine, then just dereference hw->rx_max_pktlen. Driver code can/should reference its own data directly.
>
> 2. Defining VLAN_TAG_SIZE is irrelevant if doing vlan offload.
> [Dey, Souvik] vlan offload is not mandatory. Se again still have vlan being sent up to the application. In that case we need to consider the vlan length in the Ethernet size.
The code needs to handle both vlan offload (or not), correctly. You are assuming the worst case here.
>
> 3. Virtio doesn't insert CRC, therefore CRC_LEN is irrelevant [Dey,
> Souvik] I am not sure of this. Mark commented earlier to consider this length too. Mark what do you suggest ?
Actually, the thing that matters is the size of the merge rx buf header, not the CRC.
The patch is still buggy.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v6] net/virtio: add set_mtu in virtio
2016-09-22 0:08 ` Dey, Souvik
2016-09-22 1:45 ` Stephen Hemminger
@ 2016-09-22 15:57 ` Stephen Hemminger
1 sibling, 0 replies; 17+ messages in thread
From: Stephen Hemminger @ 2016-09-22 15:57 UTC (permalink / raw)
To: Dey, Souvik; +Cc: mark.b.kavanagh, yuanhan.liu, dev
On Thu, 22 Sep 2016 00:08:38 +0000
"Dey, Souvik" <sodey@sonusnet.com> wrote:
> I am fine with the general idea of this patch but:
> 1. Calling virtio_dev_info_get is needlessly wasteful when all you want
> is to access the max packet length. Since max_rx_pktlen is always
> VIRTIO_MAX_RX_PKTLEN, please just use that.
> [Dey, Souvik] I am using the virtio_dev_info_get as in future can/may support the max_rx_pktlen as a variable to be set by the application. This will keep the changes future proof. As we need to support till 65535 instead of 9728 as the linux does.
Future proof is a myth. Don't add code based on planned future functionality
http://www.extremeprogramming.org/rules/early.html
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v6] net/virtio: add set_mtu in virtio
2016-09-22 1:45 ` Stephen Hemminger
2016-09-22 2:29 ` Dey, Souvik
@ 2016-09-23 7:53 ` Yuanhan Liu
2016-09-23 9:07 ` Kavanagh, Mark B
1 sibling, 1 reply; 17+ messages in thread
From: Yuanhan Liu @ 2016-09-23 7:53 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Dey, Souvik, mark.b.kavanagh, dev
On Wed, Sep 21, 2016 at 06:45:05PM -0700, Stephen Hemminger wrote:
> On Thu, 22 Sep 2016 00:08:38 +0000
> "Dey, Souvik" <sodey@sonusnet.com> wrote:
>
> > Answers inline.
> >
> > --
> > Regards,
> > Souvik
> >
> > -----Original Message-----
> > From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> > Sent: Wednesday, September 21, 2016 7:22 PM
> > To: Dey, Souvik <sodey@sonusnet.com>
> > Cc: mark.b.kavanagh@intel.com; yuanhan.liu@linux.intel.com; dev@dpdk.org
> > Subject: Re: [PATCH v6] net/virtio: add set_mtu in virtio
> >
> > On Wed, 21 Sep 2016 19:11:47 -0400
> > Dey <sodey@sonusnet.com> wrote:
> >
> > >
> > > +
> > > +#define VLAN_TAG_SIZE 4 /* 802.3ac tag (not DMA'd) */
> > > +
> > > +static int virtio_mtu_set(struct rte_eth_dev *dev, uint16_t mtu) {
> > > + struct rte_eth_dev_info dev_info;
> > > + uint32_t ether_hdr_len = ETHER_HDR_LEN + ETHER_CRC_LEN + VLAN_TAG_SIZE;
> > > + uint32_t frame_size = mtu + ether_hdr_len;
> > > +
> > > + virtio_dev_info_get(dev, &dev_info);
> > > +
> > > + if (mtu < ETHER_MIN_MTU || frame_size > dev_info.max_rx_pktlen) {
> > > + PMD_INIT_LOG(ERR, "MTU should be between %d and %d\n",
> > > + ETHER_MIN_MTU,
> > > + (dev_info.max_rx_pktlen - ether_hdr_len));
> > > + return -EINVAL;
> > > + }
> > > + return 0;
> > > +}
> >
> > I am fine with the general idea of this patch but:
> > 1. Calling virtio_dev_info_get is needlessly wasteful when all you want
> > is to access the max packet length. Since max_rx_pktlen is always
> > VIRTIO_MAX_RX_PKTLEN, please just use that.
> > [Dey, Souvik] I am using the virtio_dev_info_get as in future can/may support the max_rx_pktlen as a variable to be set by the application. This will keep the changes future proof. As we need to support till 65535 instead of 9728 as the linux does.
>
> Fine, then just dereference hw->rx_max_pktlen. Driver code can/should reference
> its own data directly.
Dey, maybe you could just use VIRTIO_MAX_RX_PKTLEN here, like what you
did in early versions.
> >
> > 2. Defining VLAN_TAG_SIZE is irrelevant if doing vlan offload.
> > [Dey, Souvik] vlan offload is not mandatory. Se again still have vlan being sent up to the application. In that case we need to consider the vlan length in the Ethernet size.
>
> The code needs to handle both vlan offload (or not), correctly. You are assuming
> the worst case here.
I think we are fine here to assume worst case.
> >
> > 3. Virtio doesn't insert CRC, therefore CRC_LEN is irrelevant
> > [Dey, Souvik] I am not sure of this. Mark commented earlier to consider this length too. Mark what do you suggest ?
>
> Actually, the thing that matters is the size of the merge rx buf header, not the CRC.
Right.
--yliu
>
> The patch is still buggy.
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v6] net/virtio: add set_mtu in virtio
2016-09-23 7:53 ` Yuanhan Liu
@ 2016-09-23 9:07 ` Kavanagh, Mark B
2016-09-23 15:17 ` Dey, Souvik
0 siblings, 1 reply; 17+ messages in thread
From: Kavanagh, Mark B @ 2016-09-23 9:07 UTC (permalink / raw)
To: Yuanhan Liu, Stephen Hemminger; +Cc: Dey, Souvik, dev
>Subject: Re: [PATCH v6] net/virtio: add set_mtu in virtio
>
>On Wed, Sep 21, 2016 at 06:45:05PM -0700, Stephen Hemminger wrote:
>> On Thu, 22 Sep 2016 00:08:38 +0000
>> "Dey, Souvik" <sodey@sonusnet.com> wrote:
>>
>> > Answers inline.
>> >
>> > --
>> > Regards,
>> > Souvik
>> >
>> > -----Original Message-----
>> > From: Stephen Hemminger [mailto:stephen@networkplumber.org]
>> > Sent: Wednesday, September 21, 2016 7:22 PM
>> > To: Dey, Souvik <sodey@sonusnet.com>
>> > Cc: mark.b.kavanagh@intel.com; yuanhan.liu@linux.intel.com; dev@dpdk.org
>> > Subject: Re: [PATCH v6] net/virtio: add set_mtu in virtio
>> >
>> > On Wed, 21 Sep 2016 19:11:47 -0400
>> > Dey <sodey@sonusnet.com> wrote:
>> >
>> > >
>> > > +
>> > > +#define VLAN_TAG_SIZE 4 /* 802.3ac tag (not DMA'd) */
>> > > +
>> > > +static int virtio_mtu_set(struct rte_eth_dev *dev, uint16_t mtu) {
>> > > + struct rte_eth_dev_info dev_info;
>> > > + uint32_t ether_hdr_len = ETHER_HDR_LEN + ETHER_CRC_LEN + VLAN_TAG_SIZE;
>> > > + uint32_t frame_size = mtu + ether_hdr_len;
>> > > +
>> > > + virtio_dev_info_get(dev, &dev_info);
>> > > +
>> > > + if (mtu < ETHER_MIN_MTU || frame_size > dev_info.max_rx_pktlen) {
>> > > + PMD_INIT_LOG(ERR, "MTU should be between %d and %d\n",
>> > > + ETHER_MIN_MTU,
>> > > + (dev_info.max_rx_pktlen - ether_hdr_len));
>> > > + return -EINVAL;
>> > > + }
>> > > + return 0;
>> > > +}
>> >
>> > I am fine with the general idea of this patch but:
>> > 1. Calling virtio_dev_info_get is needlessly wasteful when all you want
>> > is to access the max packet length. Since max_rx_pktlen is always
>> > VIRTIO_MAX_RX_PKTLEN, please just use that.
>> > [Dey, Souvik] I am using the virtio_dev_info_get as in future can/may support the
>max_rx_pktlen as a variable to be set by the application. This will keep the changes future
>proof. As we need to support till 65535 instead of 9728 as the linux does.
>>
>> Fine, then just dereference hw->rx_max_pktlen. Driver code can/should reference
>> its own data directly.
>
>Dey, maybe you could just use VIRTIO_MAX_RX_PKTLEN here, like what you
>did in early versions.
>
>> >
>> > 2. Defining VLAN_TAG_SIZE is irrelevant if doing vlan offload.
>> > [Dey, Souvik] vlan offload is not mandatory. Se again still have vlan being sent up to
>the application. In that case we need to consider the vlan length in the Ethernet size.
>>
>> The code needs to handle both vlan offload (or not), correctly. You are assuming
>> the worst case here.
>
>I think we are fine here to assume worst case.
>
>> >
>> > 3. Virtio doesn't insert CRC, therefore CRC_LEN is irrelevant
>> > [Dey, Souvik] I am not sure of this. Mark commented earlier to consider this length too.
>Mark what do you suggest ?
>>
>> Actually, the thing that matters is the size of the merge rx buf header, not the CRC.
>
>Right.
My comments were based on my experience with DPDK ethdev PMDs - Stephen and Yuanhan have much more experience with virtio, so I'd go with their suggestion.
>
> --yliu
>>
>> The patch is still buggy.
>>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v6] net/virtio: add set_mtu in virtio
2016-09-23 9:07 ` Kavanagh, Mark B
@ 2016-09-23 15:17 ` Dey, Souvik
2016-09-26 3:21 ` Yuanhan Liu
2016-09-27 18:56 ` Stephen Hemminger
0 siblings, 2 replies; 17+ messages in thread
From: Dey, Souvik @ 2016-09-23 15:17 UTC (permalink / raw)
To: Kavanagh, Mark B, Yuanhan Liu, Stephen Hemminger; +Cc: dev
Hi Liu/Mark/Stephen,
I have tried to modify the code with all of your latest comments. Do let me know if this looks fine or you have more comments.
Changes done :
-- max frame ize is compare to VIRTIO_MAX_RX_PKTLEN instead of dev_info.max_rx_pktlen
-- removed the CRC_LEN from the ether_len calculation and added the merge rx buf hdr len. ether_hdr_len = ETHER_HDR_LEN + VLAN_TAG_SIZE + hw->vtnet_hdr_size
-- Still retained the VLAN Size as the worst case scenario.
--
drivers/net/virtio/virtio_ethdev.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 423c597..1dbfea6 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -653,12 +653,20 @@ virtio_dev_allmulticast_disable(struct rte_eth_dev *dev)
PMD_INIT_LOG(ERR, "Failed to disable allmulticast");
}
+#define VLAN_TAG_SIZE 4 /* 802.3ac tag (not DMA'd) */
+
+static int virtio_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
+{
+ struct virtio_hw *hw = dev->data->dev_private;
+ uint32_t ether_hdr_len = ETHER_HDR_LEN + VLAN_TAG_SIZE + hw->vtnet_hdr_size;
+ uint32_t frame_size = mtu + ether_hdr_len;
+
+ if (mtu < ETHER_MIN_MTU || frame_size > VIRTIO_MAX_RX_PKTLEN ) {
+ PMD_INIT_LOG(ERR, "MTU should be between %d and %d\n",
+ ETHER_MIN_MTU, VIRTIO_MAX_RX_PKTLEN);
+ return -EINVAL;
+ }
+ return 0;
+}
/*
* dev_ops for virtio, bare necessities for basic operation
*/
@@ -677,7 +685,6 @@ static const struct eth_dev_ops virtio_eth_dev_ops = {
.allmulticast_enable = virtio_dev_allmulticast_enable,
.allmulticast_disable = virtio_dev_allmulticast_disable,
+ .mtu_set = virtio_mtu_set,
.dev_infos_get = virtio_dev_info_get,
.stats_get = virtio_dev_stats_get,
.xstats_get = virtio_dev_xstats_get,
--
Please do let me know if this looks good to you all. Thanks
--
Regards,
Souvik
-----Original Message-----
From: Kavanagh, Mark B [mailto:mark.b.kavanagh@intel.com]
Sent: Friday, September 23, 2016 5:08 AM
To: Yuanhan Liu <yuanhan.liu@linux.intel.com>; Stephen Hemminger <stephen@networkplumber.org>
Cc: Dey, Souvik <sodey@sonusnet.com>; dev@dpdk.org
Subject: RE: [PATCH v6] net/virtio: add set_mtu in virtio
>Subject: Re: [PATCH v6] net/virtio: add set_mtu in virtio
>
>On Wed, Sep 21, 2016 at 06:45:05PM -0700, Stephen Hemminger wrote:
>> On Thu, 22 Sep 2016 00:08:38 +0000
>> "Dey, Souvik" <sodey@sonusnet.com<mailto:sodey@sonusnet.com>> wrote:
>>
>> > Answers inline.
>> >
>> > --
>> > Regards,
>> > Souvik
>> >
>> > -----Original Message-----
>> > From: Stephen Hemminger [mailto:stephen@networkplumber.org]
>> > Sent: Wednesday, September 21, 2016 7:22 PM
>> > To: Dey, Souvik <sodey@sonusnet.com<mailto:sodey@sonusnet.com>>
>> > Cc: mark.b.kavanagh@intel.com<mailto:mark.b.kavanagh@intel.com>; yuanhan.liu@linux.intel.com<mailto:yuanhan.liu@linux.intel.com>;
>> > dev@dpdk.org<mailto:dev@dpdk.org>
>> > Subject: Re: [PATCH v6] net/virtio: add set_mtu in virtio
>> >
>> > On Wed, 21 Sep 2016 19:11:47 -0400
>> > Dey <sodey@sonusnet.com<mailto:sodey@sonusnet.com>> wrote:
>> >
>> > >
>> > > +
>> > > +#define VLAN_TAG_SIZE 4 /* 802.3ac tag (not DMA'd) */
>> > > +
>> > > +static int virtio_mtu_set(struct rte_eth_dev *dev, uint16_t mtu) {
>> > > + struct rte_eth_dev_info dev_info;
>> > > + uint32_t ether_hdr_len = ETHER_HDR_LEN + ETHER_CRC_LEN + VLAN_TAG_SIZE;
>> > > + uint32_t frame_size = mtu + ether_hdr_len;
>> > > +
>> > > + virtio_dev_info_get(dev, &dev_info);
>> > > +
>> > > + if (mtu < ETHER_MIN_MTU || frame_size > dev_info.max_rx_pktlen) {
>> > > + PMD_INIT_LOG(ERR, "MTU should be between %d and %d\n",
>> > > + ETHER_MIN_MTU,
>> > > + (dev_info.max_rx_pktlen - ether_hdr_len));
>> > > + return -EINVAL;
>> > > + }
>> > > + return 0;
>> > > +}
>> >
>> > I am fine with the general idea of this patch but:
>> > 1. Calling virtio_dev_info_get is needlessly wasteful when all you want
>> > is to access the max packet length. Since max_rx_pktlen is always
>> > VIRTIO_MAX_RX_PKTLEN, please just use that.
>> > [Dey, Souvik] I am using the virtio_dev_info_get as in future
>> > can/may support the
>max_rx_pktlen as a variable to be set by the application. This will
>keep the changes future proof. As we need to support till 65535 instead of 9728 as the linux does.
>>
>> Fine, then just dereference hw->rx_max_pktlen. Driver code can/should
>> reference its own data directly.
>
>Dey, maybe you could just use VIRTIO_MAX_RX_PKTLEN here, like what you
>did in early versions.
>
>> >
>> > 2. Defining VLAN_TAG_SIZE is irrelevant if doing vlan offload.
>> > [Dey, Souvik] vlan offload is not mandatory. Se again still have
>> > vlan being sent up to
>the application. In that case we need to consider the vlan length in the Ethernet size.
>>
>> The code needs to handle both vlan offload (or not), correctly. You
>> are assuming the worst case here.
>
>I think we are fine here to assume worst case.
>
>> >
>> > 3. Virtio doesn't insert CRC, therefore CRC_LEN is irrelevant
>> > [Dey, Souvik] I am not sure of this. Mark commented earlier to consider this length too.
>Mark what do you suggest ?
>>
>> Actually, the thing that matters is the size of the merge rx buf header, not the CRC.
>
>Right.
My comments were based on my experience with DPDK ethdev PMDs - Stephen and Yuanhan have much more experience with virtio, so I'd go with their suggestion.
>
> --yliu
>>
>> The patch is still buggy.
>>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v6] net/virtio: add set_mtu in virtio
2016-09-23 15:17 ` Dey, Souvik
@ 2016-09-26 3:21 ` Yuanhan Liu
2016-09-27 15:41 ` Dey, Souvik
2016-09-27 18:56 ` Stephen Hemminger
1 sibling, 1 reply; 17+ messages in thread
From: Yuanhan Liu @ 2016-09-26 3:21 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Dey, Souvik, Kavanagh, Mark B, dev
Hi Stephen,
Are you okay with this change?
--yliu
On Fri, Sep 23, 2016 at 03:17:37PM +0000, Dey, Souvik wrote:
> Hi Liu/Mark/Stephen,
>
> I have tried to modify the code with all of your latest comments.
> Do let me know if this looks fine or you have more comments.
>
>
>
> Changes done :
>
> -- max frame ize is compare to VIRTIO_MAX_RX_PKTLEN instead of
> dev_info.max_rx_pktlen
>
> -- removed the CRC_LEN from the ether_len calculation and added the merge rx
> buf hdr len. ether_hdr_len = ETHER_HDR_LEN + VLAN_TAG_SIZE + hw->vtnet_hdr_size
>
> -- Still retained the VLAN Size as the worst case scenario.
>
>
>
>
>
> --
>
> drivers/net/virtio/virtio_ethdev.c | 16 ++++++++++++++++
>
> 1 file changed, 16 insertions(+)
>
>
>
> diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/
> virtio_ethdev.c
>
> index 423c597..1dbfea6 100644
>
> --- a/drivers/net/virtio/virtio_ethdev.c
>
> +++ b/drivers/net/virtio/virtio_ethdev.c
>
> @@ -653,12 +653,20 @@ virtio_dev_allmulticast_disable(struct rte_eth_dev *dev)
>
> PMD_INIT_LOG(ERR, "Failed to disable allmulticast");
>
> }
>
>
>
> +#define VLAN_TAG_SIZE 4 /* 802.3ac tag (not DMA'd) */
>
> +
>
> +static int virtio_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
>
> +{
>
> + struct virtio_hw *hw = dev->data->dev_private;
>
> + uint32_t ether_hdr_len = ETHER_HDR_LEN + VLAN_TAG_SIZE + hw->
> vtnet_hdr_size;
>
> + uint32_t frame_size = mtu + ether_hdr_len;
>
> +
>
> + if (mtu < ETHER_MIN_MTU || frame_size > VIRTIO_MAX_RX_PKTLEN ) {
>
> + PMD_INIT_LOG(ERR, "MTU should be between %d and %d\
> n",
>
> + ETHER_MIN_MTU, VIRTIO_MAX_RX_PKTLEN);
>
> + return -EINVAL;
>
> + }
>
> + return 0;
>
> +}
>
>
>
> /*
>
> * dev_ops for virtio, bare necessities for basic operation
>
> */
>
> @@ -677,7 +685,6 @@ static const struct eth_dev_ops virtio_eth_dev_ops = {
>
> .allmulticast_enable = virtio_dev_allmulticast_enable,
>
> .allmulticast_disable = virtio_dev_allmulticast_disable,
>
> + .mtu_set = virtio_mtu_set,
>
> .dev_infos_get = virtio_dev_info_get,
>
> .stats_get = virtio_dev_stats_get,
>
> .xstats_get = virtio_dev_xstats_get,
>
> --
>
> Please do let me know if this looks good to you all. Thanks
>
>
>
> --
>
> Regards,
>
> Souvik
>
>
>
> -----Original Message-----
> From: Kavanagh, Mark B [mailto:mark.b.kavanagh@intel.com]
> Sent: Friday, September 23, 2016 5:08 AM
> To: Yuanhan Liu <yuanhan.liu@linux.intel.com>; Stephen Hemminger
> <stephen@networkplumber.org>
> Cc: Dey, Souvik <sodey@sonusnet.com>; dev@dpdk.org
> Subject: RE: [PATCH v6] net/virtio: add set_mtu in virtio
>
>
>
> >Subject: Re: [PATCH v6] net/virtio: add set_mtu in virtio
>
> >
>
> >On Wed, Sep 21, 2016 at 06:45:05PM -0700, Stephen Hemminger wrote:
>
> >> On Thu, 22 Sep 2016 00:08:38 +0000
>
> >> "Dey, Souvik" <sodey@sonusnet.com> wrote:
>
> >>
>
> >> > Answers inline.
>
> >> >
>
> >> > --
>
> >> > Regards,
>
> >> > Souvik
>
> >> >
>
> >> > -----Original Message-----
>
> >> > From: Stephen Hemminger [mailto:stephen@networkplumber.org]
>
> >> > Sent: Wednesday, September 21, 2016 7:22 PM
>
> >> > To: Dey, Souvik <sodey@sonusnet.com>
>
> >> > Cc: mark.b.kavanagh@intel.com; yuanhan.liu@linux.intel.com;
>
> >> > dev@dpdk.org
>
> >> > Subject: Re: [PATCH v6] net/virtio: add set_mtu in virtio
>
> >> >
>
> >> > On Wed, 21 Sep 2016 19:11:47 -0400
>
> >> > Dey <sodey@sonusnet.com> wrote:
>
> >> >
>
> >> > >
>
> >> > > +
>
> >> > > +#define VLAN_TAG_SIZE 4 /* 802.3ac tag (not DMA'd) */
>
> >> > > +
>
> >> > > +static int virtio_mtu_set(struct rte_eth_dev *dev, uint16_t mtu) {
>
> >> > > + struct rte_eth_dev_info dev_info;
>
> >> > > + uint32_t ether_hdr_len = ETHER_HDR_LEN + ETHER_CRC_LEN +
> VLAN_TAG_SIZE;
>
> >> > > + uint32_t frame_size = mtu + ether_hdr_len;
>
> >> > > +
>
> >> > > + virtio_dev_info_get(dev, &dev_info);
>
> >> > > +
>
> >> > > + if (mtu < ETHER_MIN_MTU || frame_size > dev_info.max_rx_pktlen)
> {
>
> >> > > + PMD_INIT_LOG(ERR, "MTU should be between %d and %d\n",
>
> >> > > + ETHER_MIN_MTU,
>
> >> > > + (dev_info.max_rx_pktlen -
> ether_hdr_len));
>
> >> > > + return -EINVAL;
>
> >> > > + }
>
> >> > > + return 0;
>
> >> > > +}
>
> >> >
>
> >> > I am fine with the general idea of this patch but:
>
> >> > 1. Calling virtio_dev_info_get is needlessly wasteful when all you want
>
> >> > is to access the max packet length. Since max_rx_pktlen is always
>
> >> > VIRTIO_MAX_RX_PKTLEN, please just use that.
>
> >> > [Dey, Souvik] I am using the virtio_dev_info_get as in future
>
> >> > can/may support the
>
> >max_rx_pktlen as a variable to be set by the application. This will
>
> >keep the changes future proof. As we need to support till 65535 instead of
> 9728 as the linux does.
>
> >>
>
> >> Fine, then just dereference hw->rx_max_pktlen. Driver code can/should
>
> >> reference its own data directly.
>
> >
>
> >Dey, maybe you could just use VIRTIO_MAX_RX_PKTLEN here, like what you
>
> >did in early versions.
>
> >
>
> >> >
>
> >> > 2. Defining VLAN_TAG_SIZE is irrelevant if doing vlan offload.
>
> >> > [Dey, Souvik] vlan offload is not mandatory. Se again still have
>
> >> > vlan being sent up to
>
> >the application. In that case we need to consider the vlan length in the
> Ethernet size.
>
> >>
>
> >> The code needs to handle both vlan offload (or not), correctly. You
>
> >> are assuming the worst case here.
>
> >
>
> >I think we are fine here to assume worst case.
>
> >
>
> >> >
>
> >> > 3. Virtio doesn't insert CRC, therefore CRC_LEN is irrelevant
>
> >> > [Dey, Souvik] I am not sure of this. Mark commented earlier to consider
> this length too.
>
> >Mark what do you suggest ?
>
> >>
>
> >> Actually, the thing that matters is the size of the merge rx buf header, not
> the CRC.
>
> >
>
> >Right.
>
>
>
> My comments were based on my experience with DPDK ethdev PMDs - Stephen and
> Yuanhan have much more experience with virtio, so I'd go with their suggestion.
>
>
>
> >
>
> > --yliu
>
> >>
>
> >> The patch is still buggy.
>
> >>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v6] net/virtio: add set_mtu in virtio
2016-09-26 3:21 ` Yuanhan Liu
@ 2016-09-27 15:41 ` Dey, Souvik
0 siblings, 0 replies; 17+ messages in thread
From: Dey, Souvik @ 2016-09-27 15:41 UTC (permalink / raw)
To: Yuanhan Liu, Stephen Hemminger; +Cc: Kavanagh, Mark B, dev
Hi All,
Any further updates/comments on this ?
--
Regards,
Souvik
-----Original Message-----
From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
Sent: Sunday, September 25, 2016 11:21 PM
To: Stephen Hemminger <stephen@networkplumber.org>
Cc: Dey, Souvik <sodey@sonusnet.com>; Kavanagh, Mark B <mark.b.kavanagh@intel.com>; dev@dpdk.org
Subject: Re: [PATCH v6] net/virtio: add set_mtu in virtio
Hi Stephen,
Are you okay with this change?
--yliu
On Fri, Sep 23, 2016 at 03:17:37PM +0000, Dey, Souvik wrote:
> Hi Liu/Mark/Stephen,
>
> I have tried to modify the code with all of your latest comments.
> Do let me know if this looks fine or you have more comments.
>
>
>
> Changes done :
>
> -- max frame ize is compare to VIRTIO_MAX_RX_PKTLEN instead of
> dev_info.max_rx_pktlen
>
> -- removed the CRC_LEN from the ether_len calculation and added the
> merge rx buf hdr len. ether_hdr_len = ETHER_HDR_LEN + VLAN_TAG_SIZE +
> hw->vtnet_hdr_size
>
> -- Still retained the VLAN Size as the worst case scenario.
>
>
>
>
>
> --
>
> drivers/net/virtio/virtio_ethdev.c | 16 ++++++++++++++++
>
> 1 file changed, 16 insertions(+)
>
>
>
> diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/
> virtio_ethdev.c
>
> index 423c597..1dbfea6 100644
>
> --- a/drivers/net/virtio/virtio_ethdev.c
>
> +++ b/drivers/net/virtio/virtio_ethdev.c
>
> @@ -653,12 +653,20 @@ virtio_dev_allmulticast_disable(struct
> rte_eth_dev *dev)
>
> PMD_INIT_LOG(ERR, "Failed to disable allmulticast");
>
> }
>
>
>
> +#define VLAN_TAG_SIZE 4 /* 802.3ac tag (not DMA'd) */
>
> +
>
> +static int virtio_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
>
> +{
>
> + struct virtio_hw *hw = dev->data->dev_private;
>
> + uint32_t ether_hdr_len = ETHER_HDR_LEN + VLAN_TAG_SIZE +
> + hw->
> vtnet_hdr_size;
>
> + uint32_t frame_size = mtu + ether_hdr_len;
>
> +
>
> + if (mtu < ETHER_MIN_MTU || frame_size >
> + VIRTIO_MAX_RX_PKTLEN ) {
>
> + PMD_INIT_LOG(ERR, "MTU should be between
> + %d and %d\
> n",
>
> + ETHER_MIN_MTU,
> + VIRTIO_MAX_RX_PKTLEN);
>
> + return -EINVAL;
>
> + }
>
> + return 0;
>
> +}
>
>
>
> /*
>
> * dev_ops for virtio, bare necessities for basic operation
>
> */
>
> @@ -677,7 +685,6 @@ static const struct eth_dev_ops virtio_eth_dev_ops
> = {
>
> .allmulticast_enable = virtio_dev_allmulticast_enable,
>
> .allmulticast_disable = virtio_dev_allmulticast_disable,
>
> + .mtu_set = virtio_mtu_set,
>
> .dev_infos_get = virtio_dev_info_get,
>
> .stats_get = virtio_dev_stats_get,
>
> .xstats_get = virtio_dev_xstats_get,
>
> --
>
> Please do let me know if this looks good to you all. Thanks
>
>
>
> --
>
> Regards,
>
> Souvik
>
>
>
> -----Original Message-----
> From: Kavanagh, Mark B [mailto:mark.b.kavanagh@intel.com]
> Sent: Friday, September 23, 2016 5:08 AM
> To: Yuanhan Liu <yuanhan.liu@linux.intel.com>; Stephen Hemminger
> <stephen@networkplumber.org>
> Cc: Dey, Souvik <sodey@sonusnet.com>; dev@dpdk.org
> Subject: RE: [PATCH v6] net/virtio: add set_mtu in virtio
>
>
>
> >Subject: Re: [PATCH v6] net/virtio: add set_mtu in virtio
>
> >
>
> >On Wed, Sep 21, 2016 at 06:45:05PM -0700, Stephen Hemminger wrote:
>
> >> On Thu, 22 Sep 2016 00:08:38 +0000
>
> >> "Dey, Souvik" <sodey@sonusnet.com> wrote:
>
> >>
>
> >> > Answers inline.
>
> >> >
>
> >> > --
>
> >> > Regards,
>
> >> > Souvik
>
> >> >
>
> >> > -----Original Message-----
>
> >> > From: Stephen Hemminger [mailto:stephen@networkplumber.org]
>
> >> > Sent: Wednesday, September 21, 2016 7:22 PM
>
> >> > To: Dey, Souvik <sodey@sonusnet.com>
>
> >> > Cc: mark.b.kavanagh@intel.com; yuanhan.liu@linux.intel.com;
>
> >> > dev@dpdk.org
>
> >> > Subject: Re: [PATCH v6] net/virtio: add set_mtu in virtio
>
> >> >
>
> >> > On Wed, 21 Sep 2016 19:11:47 -0400
>
> >> > Dey <sodey@sonusnet.com> wrote:
>
> >> >
>
> >> > >
>
> >> > > +
>
> >> > > +#define VLAN_TAG_SIZE 4 /* 802.3ac tag (not DMA'd) */
>
> >> > > +
>
> >> > > +static int virtio_mtu_set(struct rte_eth_dev *dev, uint16_t
> >> > > +mtu) {
>
> >> > > + struct rte_eth_dev_info dev_info;
>
> >> > > + uint32_t ether_hdr_len = ETHER_HDR_LEN + ETHER_CRC_LEN
> >> > > + +
> VLAN_TAG_SIZE;
>
> >> > > + uint32_t frame_size = mtu + ether_hdr_len;
>
> >> > > +
>
> >> > > + virtio_dev_info_get(dev, &dev_info);
>
> >> > > +
>
> >> > > + if (mtu < ETHER_MIN_MTU || frame_size >
> >> > > + dev_info.max_rx_pktlen)
> {
>
> >> > > + PMD_INIT_LOG(ERR, "MTU should be between %d and
> >> > > + %d\n",
>
> >> > > + ETHER_MIN_MTU,
>
> >> > > + (dev_info.max_rx_pktlen -
> ether_hdr_len));
>
> >> > > + return -EINVAL;
>
> >> > > + }
>
> >> > > + return 0;
>
> >> > > +}
>
> >> >
>
> >> > I am fine with the general idea of this patch but:
>
> >> > 1. Calling virtio_dev_info_get is needlessly wasteful when all
> >> > you want
>
> >> > is to access the max packet length. Since max_rx_pktlen is
> >> > always
>
> >> > VIRTIO_MAX_RX_PKTLEN, please just use that.
>
> >> > [Dey, Souvik] I am using the virtio_dev_info_get as in future
>
> >> > can/may support the
>
> >max_rx_pktlen as a variable to be set by the application. This will
>
> >keep the changes future proof. As we need to support till 65535
> >instead of
> 9728 as the linux does.
>
> >>
>
> >> Fine, then just dereference hw->rx_max_pktlen. Driver code
> >> can/should
>
> >> reference its own data directly.
>
> >
>
> >Dey, maybe you could just use VIRTIO_MAX_RX_PKTLEN here, like what
> >you
>
> >did in early versions.
>
> >
>
> >> >
>
> >> > 2. Defining VLAN_TAG_SIZE is irrelevant if doing vlan offload.
>
> >> > [Dey, Souvik] vlan offload is not mandatory. Se again still have
>
> >> > vlan being sent up to
>
> >the application. In that case we need to consider the vlan length in
> >the
> Ethernet size.
>
> >>
>
> >> The code needs to handle both vlan offload (or not), correctly. You
>
> >> are assuming the worst case here.
>
> >
>
> >I think we are fine here to assume worst case.
>
> >
>
> >> >
>
> >> > 3. Virtio doesn't insert CRC, therefore CRC_LEN is irrelevant
>
> >> > [Dey, Souvik] I am not sure of this. Mark commented earlier to
> >> > consider
> this length too.
>
> >Mark what do you suggest ?
>
> >>
>
> >> Actually, the thing that matters is the size of the merge rx buf
> >> header, not
> the CRC.
>
> >
>
> >Right.
>
>
>
> My comments were based on my experience with DPDK ethdev PMDs -
> Stephen and Yuanhan have much more experience with virtio, so I'd go with their suggestion.
>
>
>
> >
>
> > --yliu
>
> >>
>
> >> The patch is still buggy.
>
> >>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v6] net/virtio: add set_mtu in virtio
2016-09-23 15:17 ` Dey, Souvik
2016-09-26 3:21 ` Yuanhan Liu
@ 2016-09-27 18:56 ` Stephen Hemminger
2016-09-27 20:44 ` Dey, Souvik
1 sibling, 1 reply; 17+ messages in thread
From: Stephen Hemminger @ 2016-09-27 18:56 UTC (permalink / raw)
To: Dey, Souvik; +Cc: Kavanagh, Mark B, Yuanhan Liu, dev
On Fri, 23 Sep 2016 15:17:37 +0000
"Dey, Souvik" <sodey@sonusnet.com> wrote:
> Hi Liu/Mark/Stephen,
>
> I have tried to modify the code with all of your latest comments. Do let me know if this looks fine or you have more comments.
>
>
>
> Changes done :
>
> -- max frame ize is compare to VIRTIO_MAX_RX_PKTLEN instead of dev_info.max_rx_pktlen
>
> -- removed the CRC_LEN from the ether_len calculation and added the merge rx buf hdr len. ether_hdr_len = ETHER_HDR_LEN + VLAN_TAG_SIZE + hw->vtnet_hdr_size
>
> -- Still retained the VLAN Size as the worst case scenario.
>
>
>
>
>
> --
>
> drivers/net/virtio/virtio_ethdev.c | 16 ++++++++++++++++
>
> 1 file changed, 16 insertions(+)
>
>
>
> diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
>
> index 423c597..1dbfea6 100644
>
> --- a/drivers/net/virtio/virtio_ethdev.c
>
> +++ b/drivers/net/virtio/virtio_ethdev.c
>
> @@ -653,12 +653,20 @@ virtio_dev_allmulticast_disable(struct rte_eth_dev *dev)
>
> PMD_INIT_LOG(ERR, "Failed to disable allmulticast");
>
> }
>
>
>
> +#define VLAN_TAG_SIZE 4 /* 802.3ac tag (not DMA'd) */
>
> +
>
> +static int virtio_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
>
> +{
>
> + struct virtio_hw *hw = dev->data->dev_private;
>
> + uint32_t ether_hdr_len = ETHER_HDR_LEN + VLAN_TAG_SIZE + hw->vtnet_hdr_size;
>
> + uint32_t frame_size = mtu + ether_hdr_len;
>
> +
>
> + if (mtu < ETHER_MIN_MTU || frame_size > VIRTIO_MAX_RX_PKTLEN ) {
>
> + PMD_INIT_LOG(ERR, "MTU should be between %d and %d\n",
>
> + ETHER_MIN_MTU, VIRTIO_MAX_RX_PKTLEN);
>
> + return -EINVAL;
>
> + }
>
> + return 0;
>
> +}
>
>
>
> /*
>
> * dev_ops for virtio, bare necessities for basic operation
>
> */
>
> @@ -677,7 +685,6 @@ static const struct eth_dev_ops virtio_eth_dev_ops = {
>
> .allmulticast_enable = virtio_dev_allmulticast_enable,
>
> .allmulticast_disable = virtio_dev_allmulticast_disable,
>
> + .mtu_set = virtio_mtu_set,
>
> .dev_infos_get = virtio_dev_info_get,
>
> .stats_get = virtio_dev_stats_get,
>
> .xstats_get = virtio_dev_xstats_get,
>
> --
>
> Please do let me know if this looks good to you all. Thanks
>
>
>
> --
>
> Regards,
>
> Souvik
>
>
>
> -----Original Message-----
> From: Kavanagh, Mark B [mailto:mark.b.kavanagh@intel.com]
> Sent: Friday, September 23, 2016 5:08 AM
> To: Yuanhan Liu <yuanhan.liu@linux.intel.com>; Stephen Hemminger <stephen@networkplumber.org>
> Cc: Dey, Souvik <sodey@sonusnet.com>; dev@dpdk.org
> Subject: RE: [PATCH v6] net/virtio: add set_mtu in virtio
>
>
>
> >Subject: Re: [PATCH v6] net/virtio: add set_mtu in virtio
>
> >
>
> >On Wed, Sep 21, 2016 at 06:45:05PM -0700, Stephen Hemminger wrote:
>
> >> On Thu, 22 Sep 2016 00:08:38 +0000
>
> >> "Dey, Souvik" <sodey@sonusnet.com<mailto:sodey@sonusnet.com>> wrote:
>
> >>
>
> >> > Answers inline.
>
> >> >
>
> >> > --
>
> >> > Regards,
>
> >> > Souvik
>
> >> >
>
> >> > -----Original Message-----
>
> >> > From: Stephen Hemminger [mailto:stephen@networkplumber.org]
>
> >> > Sent: Wednesday, September 21, 2016 7:22 PM
>
> >> > To: Dey, Souvik <sodey@sonusnet.com<mailto:sodey@sonusnet.com>>
>
> >> > Cc: mark.b.kavanagh@intel.com<mailto:mark.b.kavanagh@intel.com>; yuanhan.liu@linux.intel.com<mailto:yuanhan.liu@linux.intel.com>;
>
> >> > dev@dpdk.org<mailto:dev@dpdk.org>
>
> >> > Subject: Re: [PATCH v6] net/virtio: add set_mtu in virtio
>
> >> >
>
> >> > On Wed, 21 Sep 2016 19:11:47 -0400
>
> >> > Dey <sodey@sonusnet.com<mailto:sodey@sonusnet.com>> wrote:
>
> >> >
>
> >> > >
>
> >> > > +
>
> >> > > +#define VLAN_TAG_SIZE 4 /* 802.3ac tag (not DMA'd) */
>
> >> > > +
>
> >> > > +static int virtio_mtu_set(struct rte_eth_dev *dev, uint16_t mtu) {
>
> >> > > + struct rte_eth_dev_info dev_info;
>
> >> > > + uint32_t ether_hdr_len = ETHER_HDR_LEN + ETHER_CRC_LEN + VLAN_TAG_SIZE;
>
> >> > > + uint32_t frame_size = mtu + ether_hdr_len;
>
> >> > > +
>
> >> > > + virtio_dev_info_get(dev, &dev_info);
>
> >> > > +
>
> >> > > + if (mtu < ETHER_MIN_MTU || frame_size > dev_info.max_rx_pktlen) {
>
> >> > > + PMD_INIT_LOG(ERR, "MTU should be between %d and %d\n",
>
> >> > > + ETHER_MIN_MTU,
>
> >> > > + (dev_info.max_rx_pktlen - ether_hdr_len));
>
> >> > > + return -EINVAL;
>
> >> > > + }
>
> >> > > + return 0;
>
> >> > > +}
>
> >> >
>
> >> > I am fine with the general idea of this patch but:
>
> >> > 1. Calling virtio_dev_info_get is needlessly wasteful when all you want
>
> >> > is to access the max packet length. Since max_rx_pktlen is always
>
> >> > VIRTIO_MAX_RX_PKTLEN, please just use that.
>
> >> > [Dey, Souvik] I am using the virtio_dev_info_get as in future
>
> >> > can/may support the
>
> >max_rx_pktlen as a variable to be set by the application. This will
>
> >keep the changes future proof. As we need to support till 65535 instead of 9728 as the linux does.
>
> >>
>
> >> Fine, then just dereference hw->rx_max_pktlen. Driver code can/should
>
> >> reference its own data directly.
>
> >
>
> >Dey, maybe you could just use VIRTIO_MAX_RX_PKTLEN here, like what you
>
> >did in early versions.
>
> >
>
> >> >
>
> >> > 2. Defining VLAN_TAG_SIZE is irrelevant if doing vlan offload.
>
> >> > [Dey, Souvik] vlan offload is not mandatory. Se again still have
>
> >> > vlan being sent up to
>
> >the application. In that case we need to consider the vlan length in the Ethernet size.
>
> >>
>
> >> The code needs to handle both vlan offload (or not), correctly. You
>
> >> are assuming the worst case here.
>
> >
>
> >I think we are fine here to assume worst case.
>
> >
>
> >> >
>
> >> > 3. Virtio doesn't insert CRC, therefore CRC_LEN is irrelevant
>
> >> > [Dey, Souvik] I am not sure of this. Mark commented earlier to consider this length too.
>
> >Mark what do you suggest ?
>
> >>
>
> >> Actually, the thing that matters is the size of the merge rx buf header, not the CRC.
>
This patch is good enough for now, but not fully correct.
The actual max value depends on the host negotiation protocol, ie merge rx buf
is only sometimes used.
There was some discussion on netdev mailing list to pass MTU from host to guest
in virtio, not sure where that landed.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v6] net/virtio: add set_mtu in virtio
2016-09-27 18:56 ` Stephen Hemminger
@ 2016-09-27 20:44 ` Dey, Souvik
2016-09-27 23:11 ` Yuanhan Liu
0 siblings, 1 reply; 17+ messages in thread
From: Dey, Souvik @ 2016-09-27 20:44 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Kavanagh, Mark B, Yuanhan Liu, dev
Hi Stephen,
So what should be my next steps , should I submit a v7 for this patch or you suggest otherwise.
--
Regards,
Souvik
-----Original Message-----
From: Stephen Hemminger [mailto:stephen@networkplumber.org]
Sent: Tuesday, September 27, 2016 2:57 PM
To: Dey, Souvik <sodey@sonusnet.com>
Cc: Kavanagh, Mark B <mark.b.kavanagh@intel.com>; Yuanhan Liu <yuanhan.liu@linux.intel.com>; dev@dpdk.org
Subject: Re: [PATCH v6] net/virtio: add set_mtu in virtio
On Fri, 23 Sep 2016 15:17:37 +0000
"Dey, Souvik" <sodey@sonusnet.com> wrote:
> Hi Liu/Mark/Stephen,
>
> I have tried to modify the code with all of your latest comments. Do let me know if this looks fine or you have more comments.
>
>
>
> Changes done :
>
> -- max frame ize is compare to VIRTIO_MAX_RX_PKTLEN instead of
> dev_info.max_rx_pktlen
>
> -- removed the CRC_LEN from the ether_len calculation and added the
> merge rx buf hdr len. ether_hdr_len = ETHER_HDR_LEN + VLAN_TAG_SIZE +
> hw->vtnet_hdr_size
>
> -- Still retained the VLAN Size as the worst case scenario.
>
>
>
>
>
> --
>
> drivers/net/virtio/virtio_ethdev.c | 16 ++++++++++++++++
>
> 1 file changed, 16 insertions(+)
>
>
>
> diff --git a/drivers/net/virtio/virtio_ethdev.c
> b/drivers/net/virtio/virtio_ethdev.c
>
> index 423c597..1dbfea6 100644
>
> --- a/drivers/net/virtio/virtio_ethdev.c
>
> +++ b/drivers/net/virtio/virtio_ethdev.c
>
> @@ -653,12 +653,20 @@ virtio_dev_allmulticast_disable(struct
> rte_eth_dev *dev)
>
> PMD_INIT_LOG(ERR, "Failed to disable allmulticast");
>
> }
>
>
>
> +#define VLAN_TAG_SIZE 4 /* 802.3ac tag (not DMA'd) */
>
> +
>
> +static int virtio_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
>
> +{
>
> + struct virtio_hw *hw = dev->data->dev_private;
>
> + uint32_t ether_hdr_len = ETHER_HDR_LEN + VLAN_TAG_SIZE +
> + hw->vtnet_hdr_size;
>
> + uint32_t frame_size = mtu + ether_hdr_len;
>
> +
>
> + if (mtu < ETHER_MIN_MTU || frame_size >
> + VIRTIO_MAX_RX_PKTLEN ) {
>
> + PMD_INIT_LOG(ERR, "MTU should be between
> + %d and %d\n",
>
> + ETHER_MIN_MTU,
> + VIRTIO_MAX_RX_PKTLEN);
>
> + return -EINVAL;
>
> + }
>
> + return 0;
>
> +}
>
>
>
> /*
>
> * dev_ops for virtio, bare necessities for basic operation
>
> */
>
> @@ -677,7 +685,6 @@ static const struct eth_dev_ops virtio_eth_dev_ops
> = {
>
> .allmulticast_enable = virtio_dev_allmulticast_enable,
>
> .allmulticast_disable = virtio_dev_allmulticast_disable,
>
> + .mtu_set = virtio_mtu_set,
>
> .dev_infos_get = virtio_dev_info_get,
>
> .stats_get = virtio_dev_stats_get,
>
> .xstats_get = virtio_dev_xstats_get,
>
> --
>
> Please do let me know if this looks good to you all. Thanks
>
>
>
> --
>
> Regards,
>
> Souvik
>
>
>
> -----Original Message-----
> From: Kavanagh, Mark B [mailto:mark.b.kavanagh@intel.com]
> Sent: Friday, September 23, 2016 5:08 AM
> To: Yuanhan Liu <yuanhan.liu@linux.intel.com>; Stephen Hemminger
> <stephen@networkplumber.org>
> Cc: Dey, Souvik <sodey@sonusnet.com>; dev@dpdk.org
> Subject: RE: [PATCH v6] net/virtio: add set_mtu in virtio
>
>
>
> >Subject: Re: [PATCH v6] net/virtio: add set_mtu in virtio
>
> >
>
> >On Wed, Sep 21, 2016 at 06:45:05PM -0700, Stephen Hemminger wrote:
>
> >> On Thu, 22 Sep 2016 00:08:38 +0000
>
> >> "Dey, Souvik" <sodey@sonusnet.com<mailto:sodey@sonusnet.com>> wrote:
>
> >>
>
> >> > Answers inline.
>
> >> >
>
> >> > --
>
> >> > Regards,
>
> >> > Souvik
>
> >> >
>
> >> > -----Original Message-----
>
> >> > From: Stephen Hemminger [mailto:stephen@networkplumber.org]
>
> >> > Sent: Wednesday, September 21, 2016 7:22 PM
>
> >> > To: Dey, Souvik <sodey@sonusnet.com<mailto:sodey@sonusnet.com>>
>
> >> > Cc: mark.b.kavanagh@intel.com<mailto:mark.b.kavanagh@intel.com>;
> >> > yuanhan.liu@linux.intel.com<mailto:yuanhan.liu@linux.intel.com>;
>
> >> > dev@dpdk.org<mailto:dev@dpdk.org>
>
> >> > Subject: Re: [PATCH v6] net/virtio: add set_mtu in virtio
>
> >> >
>
> >> > On Wed, 21 Sep 2016 19:11:47 -0400
>
> >> > Dey <sodey@sonusnet.com<mailto:sodey@sonusnet.com>> wrote:
>
> >> >
>
> >> > >
>
> >> > > +
>
> >> > > +#define VLAN_TAG_SIZE 4 /* 802.3ac tag (not DMA'd) */
>
> >> > > +
>
> >> > > +static int virtio_mtu_set(struct rte_eth_dev *dev, uint16_t
> >> > > +mtu) {
>
> >> > > + struct rte_eth_dev_info dev_info;
>
> >> > > + uint32_t ether_hdr_len = ETHER_HDR_LEN + ETHER_CRC_LEN
> >> > > + + VLAN_TAG_SIZE;
>
> >> > > + uint32_t frame_size = mtu + ether_hdr_len;
>
> >> > > +
>
> >> > > + virtio_dev_info_get(dev, &dev_info);
>
> >> > > +
>
> >> > > + if (mtu < ETHER_MIN_MTU || frame_size >
> >> > > + dev_info.max_rx_pktlen) {
>
> >> > > + PMD_INIT_LOG(ERR, "MTU should be between %d and
> >> > > + %d\n",
>
> >> > > + ETHER_MIN_MTU,
>
> >> > > + (dev_info.max_rx_pktlen -
> >> > > + ether_hdr_len));
>
> >> > > + return -EINVAL;
>
> >> > > + }
>
> >> > > + return 0;
>
> >> > > +}
>
> >> >
>
> >> > I am fine with the general idea of this patch but:
>
> >> > 1. Calling virtio_dev_info_get is needlessly wasteful when all
> >> > you want
>
> >> > is to access the max packet length. Since max_rx_pktlen is
> >> > always
>
> >> > VIRTIO_MAX_RX_PKTLEN, please just use that.
>
> >> > [Dey, Souvik] I am using the virtio_dev_info_get as in future
>
> >> > can/may support the
>
> >max_rx_pktlen as a variable to be set by the application. This will
>
> >keep the changes future proof. As we need to support till 65535 instead of 9728 as the linux does.
>
> >>
>
> >> Fine, then just dereference hw->rx_max_pktlen. Driver code
> >> can/should
>
> >> reference its own data directly.
>
> >
>
> >Dey, maybe you could just use VIRTIO_MAX_RX_PKTLEN here, like what
> >you
>
> >did in early versions.
>
> >
>
> >> >
>
> >> > 2. Defining VLAN_TAG_SIZE is irrelevant if doing vlan offload.
>
> >> > [Dey, Souvik] vlan offload is not mandatory. Se again still have
>
> >> > vlan being sent up to
>
> >the application. In that case we need to consider the vlan length in the Ethernet size.
>
> >>
>
> >> The code needs to handle both vlan offload (or not), correctly. You
>
> >> are assuming the worst case here.
>
> >
>
> >I think we are fine here to assume worst case.
>
> >
>
> >> >
>
> >> > 3. Virtio doesn't insert CRC, therefore CRC_LEN is irrelevant
>
> >> > [Dey, Souvik] I am not sure of this. Mark commented earlier to consider this length too.
>
> >Mark what do you suggest ?
>
> >>
>
> >> Actually, the thing that matters is the size of the merge rx buf header, not the CRC.
>
This patch is good enough for now, but not fully correct.
The actual max value depends on the host negotiation protocol, ie merge rx buf is only sometimes used.
There was some discussion on netdev mailing list to pass MTU from host to guest in virtio, not sure where that landed.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v6] net/virtio: add set_mtu in virtio
2016-09-27 20:44 ` Dey, Souvik
@ 2016-09-27 23:11 ` Yuanhan Liu
2016-09-28 15:31 ` Dey, Souvik
0 siblings, 1 reply; 17+ messages in thread
From: Yuanhan Liu @ 2016-09-27 23:11 UTC (permalink / raw)
To: Dey, Souvik; +Cc: Stephen Hemminger, Kavanagh, Mark B, dev
On Tue, Sep 27, 2016 at 08:44:20PM +0000, Dey, Souvik wrote:
> Hi Stephen,
> So what should be my next steps , should I submit a v7 for this patch or you suggest otherwise.
Yes, please. Another note is please don't use white space for
indentation, use TAB instead.
-yliu
>
> --
> Regards,
> Souvik
>
> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Tuesday, September 27, 2016 2:57 PM
> To: Dey, Souvik <sodey@sonusnet.com>
> Cc: Kavanagh, Mark B <mark.b.kavanagh@intel.com>; Yuanhan Liu <yuanhan.liu@linux.intel.com>; dev@dpdk.org
> Subject: Re: [PATCH v6] net/virtio: add set_mtu in virtio
>
> On Fri, 23 Sep 2016 15:17:37 +0000
> "Dey, Souvik" <sodey@sonusnet.com> wrote:
>
> > Hi Liu/Mark/Stephen,
> >
> > I have tried to modify the code with all of your latest comments. Do let me know if this looks fine or you have more comments.
> >
> >
> >
> > Changes done :
> >
> > -- max frame ize is compare to VIRTIO_MAX_RX_PKTLEN instead of
> > dev_info.max_rx_pktlen
> >
> > -- removed the CRC_LEN from the ether_len calculation and added the
> > merge rx buf hdr len. ether_hdr_len = ETHER_HDR_LEN + VLAN_TAG_SIZE +
> > hw->vtnet_hdr_size
> >
> > -- Still retained the VLAN Size as the worst case scenario.
> >
> >
> >
> >
> >
> > --
> >
> > drivers/net/virtio/virtio_ethdev.c | 16 ++++++++++++++++
> >
> > 1 file changed, 16 insertions(+)
> >
> >
> >
> > diff --git a/drivers/net/virtio/virtio_ethdev.c
> > b/drivers/net/virtio/virtio_ethdev.c
> >
> > index 423c597..1dbfea6 100644
> >
> > --- a/drivers/net/virtio/virtio_ethdev.c
> >
> > +++ b/drivers/net/virtio/virtio_ethdev.c
> >
> > @@ -653,12 +653,20 @@ virtio_dev_allmulticast_disable(struct
> > rte_eth_dev *dev)
> >
> > PMD_INIT_LOG(ERR, "Failed to disable allmulticast");
> >
> > }
> >
> >
> >
> > +#define VLAN_TAG_SIZE 4 /* 802.3ac tag (not DMA'd) */
> >
> > +
> >
> > +static int virtio_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
> >
> > +{
> >
> > + struct virtio_hw *hw = dev->data->dev_private;
> >
> > + uint32_t ether_hdr_len = ETHER_HDR_LEN + VLAN_TAG_SIZE +
> > + hw->vtnet_hdr_size;
> >
> > + uint32_t frame_size = mtu + ether_hdr_len;
> >
> > +
> >
> > + if (mtu < ETHER_MIN_MTU || frame_size >
> > + VIRTIO_MAX_RX_PKTLEN ) {
> >
> > + PMD_INIT_LOG(ERR, "MTU should be between
> > + %d and %d\n",
> >
> > + ETHER_MIN_MTU,
> > + VIRTIO_MAX_RX_PKTLEN);
> >
> > + return -EINVAL;
> >
> > + }
> >
> > + return 0;
> >
> > +}
> >
> >
> >
> > /*
> >
> > * dev_ops for virtio, bare necessities for basic operation
> >
> > */
> >
> > @@ -677,7 +685,6 @@ static const struct eth_dev_ops virtio_eth_dev_ops
> > = {
> >
> > .allmulticast_enable = virtio_dev_allmulticast_enable,
> >
> > .allmulticast_disable = virtio_dev_allmulticast_disable,
> >
> > + .mtu_set = virtio_mtu_set,
> >
> > .dev_infos_get = virtio_dev_info_get,
> >
> > .stats_get = virtio_dev_stats_get,
> >
> > .xstats_get = virtio_dev_xstats_get,
> >
> > --
> >
> > Please do let me know if this looks good to you all. Thanks
> >
> >
> >
> > --
> >
> > Regards,
> >
> > Souvik
> >
> >
> >
> > -----Original Message-----
> > From: Kavanagh, Mark B [mailto:mark.b.kavanagh@intel.com]
> > Sent: Friday, September 23, 2016 5:08 AM
> > To: Yuanhan Liu <yuanhan.liu@linux.intel.com>; Stephen Hemminger
> > <stephen@networkplumber.org>
> > Cc: Dey, Souvik <sodey@sonusnet.com>; dev@dpdk.org
> > Subject: RE: [PATCH v6] net/virtio: add set_mtu in virtio
> >
> >
> >
> > >Subject: Re: [PATCH v6] net/virtio: add set_mtu in virtio
> >
> > >
> >
> > >On Wed, Sep 21, 2016 at 06:45:05PM -0700, Stephen Hemminger wrote:
> >
> > >> On Thu, 22 Sep 2016 00:08:38 +0000
> >
> > >> "Dey, Souvik" <sodey@sonusnet.com<mailto:sodey@sonusnet.com>> wrote:
> >
> > >>
> >
> > >> > Answers inline.
> >
> > >> >
> >
> > >> > --
> >
> > >> > Regards,
> >
> > >> > Souvik
> >
> > >> >
> >
> > >> > -----Original Message-----
> >
> > >> > From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> >
> > >> > Sent: Wednesday, September 21, 2016 7:22 PM
> >
> > >> > To: Dey, Souvik <sodey@sonusnet.com<mailto:sodey@sonusnet.com>>
> >
> > >> > Cc: mark.b.kavanagh@intel.com<mailto:mark.b.kavanagh@intel.com>;
> > >> > yuanhan.liu@linux.intel.com<mailto:yuanhan.liu@linux.intel.com>;
> >
> > >> > dev@dpdk.org<mailto:dev@dpdk.org>
> >
> > >> > Subject: Re: [PATCH v6] net/virtio: add set_mtu in virtio
> >
> > >> >
> >
> > >> > On Wed, 21 Sep 2016 19:11:47 -0400
> >
> > >> > Dey <sodey@sonusnet.com<mailto:sodey@sonusnet.com>> wrote:
> >
> > >> >
> >
> > >> > >
> >
> > >> > > +
> >
> > >> > > +#define VLAN_TAG_SIZE 4 /* 802.3ac tag (not DMA'd) */
> >
> > >> > > +
> >
> > >> > > +static int virtio_mtu_set(struct rte_eth_dev *dev, uint16_t
> > >> > > +mtu) {
> >
> > >> > > + struct rte_eth_dev_info dev_info;
> >
> > >> > > + uint32_t ether_hdr_len = ETHER_HDR_LEN + ETHER_CRC_LEN
> > >> > > + + VLAN_TAG_SIZE;
> >
> > >> > > + uint32_t frame_size = mtu + ether_hdr_len;
> >
> > >> > > +
> >
> > >> > > + virtio_dev_info_get(dev, &dev_info);
> >
> > >> > > +
> >
> > >> > > + if (mtu < ETHER_MIN_MTU || frame_size >
> > >> > > + dev_info.max_rx_pktlen) {
> >
> > >> > > + PMD_INIT_LOG(ERR, "MTU should be between %d and
> > >> > > + %d\n",
> >
> > >> > > + ETHER_MIN_MTU,
> >
> > >> > > + (dev_info.max_rx_pktlen -
> > >> > > + ether_hdr_len));
> >
> > >> > > + return -EINVAL;
> >
> > >> > > + }
> >
> > >> > > + return 0;
> >
> > >> > > +}
> >
> > >> >
> >
> > >> > I am fine with the general idea of this patch but:
> >
> > >> > 1. Calling virtio_dev_info_get is needlessly wasteful when all
> > >> > you want
> >
> > >> > is to access the max packet length. Since max_rx_pktlen is
> > >> > always
> >
> > >> > VIRTIO_MAX_RX_PKTLEN, please just use that.
> >
> > >> > [Dey, Souvik] I am using the virtio_dev_info_get as in future
> >
> > >> > can/may support the
> >
> > >max_rx_pktlen as a variable to be set by the application. This will
> >
> > >keep the changes future proof. As we need to support till 65535 instead of 9728 as the linux does.
> >
> > >>
> >
> > >> Fine, then just dereference hw->rx_max_pktlen. Driver code
> > >> can/should
> >
> > >> reference its own data directly.
> >
> > >
> >
> > >Dey, maybe you could just use VIRTIO_MAX_RX_PKTLEN here, like what
> > >you
> >
> > >did in early versions.
> >
> > >
> >
> > >> >
> >
> > >> > 2. Defining VLAN_TAG_SIZE is irrelevant if doing vlan offload.
> >
> > >> > [Dey, Souvik] vlan offload is not mandatory. Se again still have
> >
> > >> > vlan being sent up to
> >
> > >the application. In that case we need to consider the vlan length in the Ethernet size.
> >
> > >>
> >
> > >> The code needs to handle both vlan offload (or not), correctly. You
> >
> > >> are assuming the worst case here.
> >
> > >
> >
> > >I think we are fine here to assume worst case.
> >
> > >
> >
> > >> >
> >
> > >> > 3. Virtio doesn't insert CRC, therefore CRC_LEN is irrelevant
> >
> > >> > [Dey, Souvik] I am not sure of this. Mark commented earlier to consider this length too.
> >
> > >Mark what do you suggest ?
> >
> > >>
> >
> > >> Actually, the thing that matters is the size of the merge rx buf header, not the CRC.
> >
>
> This patch is good enough for now, but not fully correct.
> The actual max value depends on the host negotiation protocol, ie merge rx buf is only sometimes used.
>
> There was some discussion on netdev mailing list to pass MTU from host to guest in virtio, not sure where that landed.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v6] net/virtio: add set_mtu in virtio
2016-09-27 23:11 ` Yuanhan Liu
@ 2016-09-28 15:31 ` Dey, Souvik
0 siblings, 0 replies; 17+ messages in thread
From: Dey, Souvik @ 2016-09-28 15:31 UTC (permalink / raw)
To: Yuanhan Liu; +Cc: Stephen Hemminger, Kavanagh, Mark B, dev
OK sure will do that.
-----Original Message-----
From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
Sent: Tuesday, September 27, 2016 7:12 PM
To: Dey, Souvik <sodey@sonusnet.com>
Cc: Stephen Hemminger <stephen@networkplumber.org>; Kavanagh, Mark B <mark.b.kavanagh@intel.com>; dev@dpdk.org
Subject: Re: [PATCH v6] net/virtio: add set_mtu in virtio
On Tue, Sep 27, 2016 at 08:44:20PM +0000, Dey, Souvik wrote:
> Hi Stephen,
> So what should be my next steps , should I submit a v7 for this patch or you suggest otherwise.
Yes, please. Another note is please don't use white space for indentation, use TAB instead.
-yliu
>
> --
> Regards,
> Souvik
>
> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Tuesday, September 27, 2016 2:57 PM
> To: Dey, Souvik <sodey@sonusnet.com>
> Cc: Kavanagh, Mark B <mark.b.kavanagh@intel.com>; Yuanhan Liu
> <yuanhan.liu@linux.intel.com>; dev@dpdk.org
> Subject: Re: [PATCH v6] net/virtio: add set_mtu in virtio
>
> On Fri, 23 Sep 2016 15:17:37 +0000
> "Dey, Souvik" <sodey@sonusnet.com> wrote:
>
> > Hi Liu/Mark/Stephen,
> >
> > I have tried to modify the code with all of your latest comments. Do let me know if this looks fine or you have more comments.
> >
> >
> >
> > Changes done :
> >
> > -- max frame ize is compare to VIRTIO_MAX_RX_PKTLEN instead of
> > dev_info.max_rx_pktlen
> >
> > -- removed the CRC_LEN from the ether_len calculation and added the
> > merge rx buf hdr len. ether_hdr_len = ETHER_HDR_LEN + VLAN_TAG_SIZE
> > +
> > hw->vtnet_hdr_size
> >
> > -- Still retained the VLAN Size as the worst case scenario.
> >
> >
> >
> >
> >
> > --
> >
> > drivers/net/virtio/virtio_ethdev.c | 16 ++++++++++++++++
> >
> > 1 file changed, 16 insertions(+)
> >
> >
> >
> > diff --git a/drivers/net/virtio/virtio_ethdev.c
> > b/drivers/net/virtio/virtio_ethdev.c
> >
> > index 423c597..1dbfea6 100644
> >
> > --- a/drivers/net/virtio/virtio_ethdev.c
> >
> > +++ b/drivers/net/virtio/virtio_ethdev.c
> >
> > @@ -653,12 +653,20 @@ virtio_dev_allmulticast_disable(struct
> > rte_eth_dev *dev)
> >
> > PMD_INIT_LOG(ERR, "Failed to disable allmulticast");
> >
> > }
> >
> >
> >
> > +#define VLAN_TAG_SIZE 4 /* 802.3ac tag (not DMA'd) */
> >
> > +
> >
> > +static int virtio_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
> >
> > +{
> >
> > + struct virtio_hw *hw = dev->data->dev_private;
> >
> > + uint32_t ether_hdr_len = ETHER_HDR_LEN + VLAN_TAG_SIZE
> > + +
> > + hw->vtnet_hdr_size;
> >
> > + uint32_t frame_size = mtu + ether_hdr_len;
> >
> > +
> >
> > + if (mtu < ETHER_MIN_MTU || frame_size >
> > + VIRTIO_MAX_RX_PKTLEN ) {
> >
> > + PMD_INIT_LOG(ERR, "MTU should be between
> > + %d and %d\n",
> >
> > + ETHER_MIN_MTU,
> > + VIRTIO_MAX_RX_PKTLEN);
> >
> > + return -EINVAL;
> >
> > + }
> >
> > + return 0;
> >
> > +}
> >
> >
> >
> > /*
> >
> > * dev_ops for virtio, bare necessities for basic operation
> >
> > */
> >
> > @@ -677,7 +685,6 @@ static const struct eth_dev_ops
> > virtio_eth_dev_ops = {
> >
> > .allmulticast_enable = virtio_dev_allmulticast_enable,
> >
> > .allmulticast_disable = virtio_dev_allmulticast_disable,
> >
> > + .mtu_set = virtio_mtu_set,
> >
> > .dev_infos_get = virtio_dev_info_get,
> >
> > .stats_get = virtio_dev_stats_get,
> >
> > .xstats_get = virtio_dev_xstats_get,
> >
> > --
> >
> > Please do let me know if this looks good to you all. Thanks
> >
> >
> >
> > --
> >
> > Regards,
> >
> > Souvik
> >
> >
> >
> > -----Original Message-----
> > From: Kavanagh, Mark B [mailto:mark.b.kavanagh@intel.com]
> > Sent: Friday, September 23, 2016 5:08 AM
> > To: Yuanhan Liu <yuanhan.liu@linux.intel.com>; Stephen Hemminger
> > <stephen@networkplumber.org>
> > Cc: Dey, Souvik <sodey@sonusnet.com>; dev@dpdk.org
> > Subject: RE: [PATCH v6] net/virtio: add set_mtu in virtio
> >
> >
> >
> > >Subject: Re: [PATCH v6] net/virtio: add set_mtu in virtio
> >
> > >
> >
> > >On Wed, Sep 21, 2016 at 06:45:05PM -0700, Stephen Hemminger wrote:
> >
> > >> On Thu, 22 Sep 2016 00:08:38 +0000
> >
> > >> "Dey, Souvik" <sodey@sonusnet.com<mailto:sodey@sonusnet.com>> wrote:
> >
> > >>
> >
> > >> > Answers inline.
> >
> > >> >
> >
> > >> > --
> >
> > >> > Regards,
> >
> > >> > Souvik
> >
> > >> >
> >
> > >> > -----Original Message-----
> >
> > >> > From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> >
> > >> > Sent: Wednesday, September 21, 2016 7:22 PM
> >
> > >> > To: Dey, Souvik <sodey@sonusnet.com<mailto:sodey@sonusnet.com>>
> >
> > >> > Cc:
> > >> > mark.b.kavanagh@intel.com<mailto:mark.b.kavanagh@intel.com>;
> > >> > yuanhan.liu@linux.intel.com<mailto:yuanhan.liu@linux.intel.com>
> > >> > ;
> >
> > >> > dev@dpdk.org<mailto:dev@dpdk.org>
> >
> > >> > Subject: Re: [PATCH v6] net/virtio: add set_mtu in virtio
> >
> > >> >
> >
> > >> > On Wed, 21 Sep 2016 19:11:47 -0400
> >
> > >> > Dey <sodey@sonusnet.com<mailto:sodey@sonusnet.com>> wrote:
> >
> > >> >
> >
> > >> > >
> >
> > >> > > +
> >
> > >> > > +#define VLAN_TAG_SIZE 4 /* 802.3ac tag (not DMA'd) */
> >
> > >> > > +
> >
> > >> > > +static int virtio_mtu_set(struct rte_eth_dev *dev, uint16_t
> > >> > > +mtu) {
> >
> > >> > > + struct rte_eth_dev_info dev_info;
> >
> > >> > > + uint32_t ether_hdr_len = ETHER_HDR_LEN +
> > >> > > + ETHER_CRC_LEN
> > >> > > + + VLAN_TAG_SIZE;
> >
> > >> > > + uint32_t frame_size = mtu + ether_hdr_len;
> >
> > >> > > +
> >
> > >> > > + virtio_dev_info_get(dev, &dev_info);
> >
> > >> > > +
> >
> > >> > > + if (mtu < ETHER_MIN_MTU || frame_size >
> > >> > > + dev_info.max_rx_pktlen) {
> >
> > >> > > + PMD_INIT_LOG(ERR, "MTU should be between %d
> > >> > > + and %d\n",
> >
> > >> > > + ETHER_MIN_MTU,
> >
> > >> > > + (dev_info.max_rx_pktlen -
> > >> > > + ether_hdr_len));
> >
> > >> > > + return -EINVAL;
> >
> > >> > > + }
> >
> > >> > > + return 0;
> >
> > >> > > +}
> >
> > >> >
> >
> > >> > I am fine with the general idea of this patch but:
> >
> > >> > 1. Calling virtio_dev_info_get is needlessly wasteful when
> > >> > all you want
> >
> > >> > is to access the max packet length. Since max_rx_pktlen is
> > >> > always
> >
> > >> > VIRTIO_MAX_RX_PKTLEN, please just use that.
> >
> > >> > [Dey, Souvik] I am using the virtio_dev_info_get as in future
> >
> > >> > can/may support the
> >
> > >max_rx_pktlen as a variable to be set by the application. This
> > >will
> >
> > >keep the changes future proof. As we need to support till 65535 instead of 9728 as the linux does.
> >
> > >>
> >
> > >> Fine, then just dereference hw->rx_max_pktlen. Driver code
> > >> can/should
> >
> > >> reference its own data directly.
> >
> > >
> >
> > >Dey, maybe you could just use VIRTIO_MAX_RX_PKTLEN here, like what
> > >you
> >
> > >did in early versions.
> >
> > >
> >
> > >> >
> >
> > >> > 2. Defining VLAN_TAG_SIZE is irrelevant if doing vlan offload.
> >
> > >> > [Dey, Souvik] vlan offload is not mandatory. Se again still
> > >> > have
> >
> > >> > vlan being sent up to
> >
> > >the application. In that case we need to consider the vlan length in the Ethernet size.
> >
> > >>
> >
> > >> The code needs to handle both vlan offload (or not), correctly.
> > >> You
> >
> > >> are assuming the worst case here.
> >
> > >
> >
> > >I think we are fine here to assume worst case.
> >
> > >
> >
> > >> >
> >
> > >> > 3. Virtio doesn't insert CRC, therefore CRC_LEN is irrelevant
> >
> > >> > [Dey, Souvik] I am not sure of this. Mark commented earlier to consider this length too.
> >
> > >Mark what do you suggest ?
> >
> > >>
> >
> > >> Actually, the thing that matters is the size of the merge rx buf header, not the CRC.
> >
>
> This patch is good enough for now, but not fully correct.
> The actual max value depends on the host negotiation protocol, ie merge rx buf is only sometimes used.
>
> There was some discussion on netdev mailing list to pass MTU from host to guest in virtio, not sure where that landed.
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v6] net/virtio: add set_mtu in virtio
2016-09-16 17:13 [PATCH v5]net/virtio: add mtu set " souvikdey33
@ 2016-09-22 13:56 ` Dey
0 siblings, 0 replies; 17+ messages in thread
From: Dey @ 2016-09-22 13:56 UTC (permalink / raw)
To: mark.b.kavanagh, yuanhan.liu, stephen, dev; +Cc: Dey
Virtio interfaces do not currently allow the user to specify a particular
Maximum Transmission Unit (MTU).Consequently, the MTU of Virtio interfaces
is typically set to the Ethernet default value of 1500.
This is problematic in the case of cloud deployments, in which a specific
(and potentially non-standard) MTU needs to be set by a DHCP server, which
needs to be honored by all interfaces across the traffic path.To acheive
this Virtio interfaces should support setting of MTU.
In case when GRE/VXLAN tunneling is used for internal communication, there
will be an overhead added by the infrastructure in the packet over and
above the ETHER MTU of 1518. So to take care of this overhead in these
cases the DHCP server corrects the L3 MTU to 1454. But since virtio
interfaces was not having the MTU set functionality that MTU sent by the
DHCP server was ignored and the instance will still send packets with 1500
MTU which after encapsulation will become more than 1518 and eventually
gets dropped in the infrastructure.
By adding an additional 'set_mtu' function to the Virtio driver, we can
honor the MTU sent by the DHCP server. The dhcp server/controller can
then leverage this 'set_mtu' functionality to resolve the above
mentioned issue of packets getting dropped due to incorrect size.
Signed-off-by: Souvik Dey <sodey@sonusnet.com>
---
Changes in v6:
- Description of change corrected
- Corrected the identations
- Corrected the subject line too
- The From line was also not correct
- Re-submitting as the below patch was not proper
Changes in v5:
- Fix log message for out-of-bounds MTU parameter in virtio_mtu_set
- Calculate frame size, based on 'mtu' parameter
- Corrected the upper bound and lower bound checks in virtio_mtu_set
Changes in v4: Incorporated review comments.
Changes in v3: Corrected few style errors as reported by sys-stv.
Changes in v2: Incorporated review comments.
drivers/net/virtio/virtio_ethdev.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 423c597..1dbfea6 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -653,12 +653,20 @@ virtio_dev_allmulticast_disable(struct rte_eth_dev *dev)
PMD_INIT_LOG(ERR, "Failed to disable allmulticast");
}
+#define VLAN_TAG_SIZE 4 /* 802.3ac tag (not DMA'd) */
+
+static int virtio_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
+{
+ struct rte_eth_dev_info dev_info;
+ uint32_t ether_hdr_len = ETHER_HDR_LEN + ETHER_CRC_LEN + VLAN_TAG_SIZE;
+ uint32_t frame_size = mtu + ether_hdr_len;
+
+ virtio_dev_info_get(dev, &dev_info);
+
+ if (mtu < ETHER_MIN_MTU || frame_size > dev_info.max_rx_pktlen) {
+ PMD_INIT_LOG(ERR, "MTU should be between %d and %d\n",
+ ETHER_MIN_MTU,
+ (dev_info.max_rx_pktlen - ether_hdr_len));
+ return -EINVAL;
+ }
+ return 0;
+}
/*
* dev_ops for virtio, bare necessities for basic operation
*/
@@ -677,7 +685,6 @@ static const struct eth_dev_ops virtio_eth_dev_ops = {
.allmulticast_enable = virtio_dev_allmulticast_enable,
.allmulticast_disable = virtio_dev_allmulticast_disable,
+ .mtu_set = virtio_mtu_set,
.dev_infos_get = virtio_dev_info_get,
.stats_get = virtio_dev_stats_get,
.xstats_get = virtio_dev_xstats_get,
--
2.9.3.windows.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v6] net/virtio: add set_mtu in virtio
@ 2016-09-21 21:10 Dey
0 siblings, 0 replies; 17+ messages in thread
From: Dey @ 2016-09-21 21:10 UTC (permalink / raw)
To: mark.b.kavanagh, yuanhan.liu, stephen, dev; +Cc: souvikdey33
From: souvikdey33 <sodey@sonusnet.com>
Virtio interfaces do not currently allow the user to specify a particular
Maximum Transmission Unit (MTU).Consequently, the MTU of Virtio interfaces
is typically set to the Ethernet default value of 1500.
This is problematic in the case of cloud deployments, in which a specific
(and potentially non-standard) MTU needs to be set by a DHCP server, which
needs to be honored by all interfaces across the traffic path.To acheive
this Virtio interfaces should support setting of MTU.
In case when GRE/VXLAN tunneling is used for internal communication, there
will be an overhead added by the infrastructure in the packet over and
above the ETHER MTU of 1518. So to take care of this overhead in these
cases the DHCP server corrects the L3 MTU to 1454. But since virtio
interfaces was not having the MTU set functionality that MTU sent by the
DHCP server was ignored and the instance will still send packets with 1500
MTU which after encapsulation will become more than 1518 and eventually
gets dropped in the infrastructure.
By adding an additional 'set_mtu' function to the Virtio driver, we can
honor the MTU sent by the DHCP server. The dhcp server/controller can
then leverage this 'set_mtu' functionality to resolve the above
mentioned issue of packets getting dropped due to incorrect size.
Signed-off-by: Souvik Dey <sodey@sonusnet.com>
---
Changes in v6:
- Description of change corrected
- Corrected the identations
- Corrected the subject line too
- The From line was also not correct
Changes in v5:
- Fix log message for out-of-bounds MTU parameter in virtio_mtu_set
- Calculate frame size, based on 'mtu' parameter
- Corrected the upper bound and lower bound checks in virtio_mtu_set
Changes in v4: Incorporated review comments.
Changes in v3: Corrected few style errors as reported by sys-stv.
Changes in v2: Incorporated review comments.
drivers/net/virtio/virtio_ethdev.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 423c597..1dbfea6 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
+
+#define VLAN_TAG_SIZE 4 /* 802.3ac tag (not DMA'd) */
+
+static int virtio_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
+{
+ struct rte_eth_dev_info dev_info;
+ uint32_t ether_hdr_len = ETHER_HDR_LEN + ETHER_CRC_LEN + VLAN_TAG_SIZE;
+ uint32_t frame_size = mtu + ether_hdr_len;
+
+ virtio_dev_info_get(dev, &dev_info);
+
+ if (mtu < ETHER_MIN_MTU || frame_size > dev_info.max_rx_pktlen) {
+ PMD_INIT_LOG(ERR, "MTU should be between %d and %d\n",
+ ETHER_MIN_MTU,
+ (dev_info.max_rx_pktlen - ether_hdr_len));
+ return -EINVAL;
+ }
+ return 0;
+}
/*
* dev_ops for virtio, bare necessities for basic operation
*/
@@ -677,7 +685,6 @@ static const struct eth_dev_ops virtio_eth_dev_ops = {
.allmulticast_enable = virtio_dev_allmulticast_enable,
.allmulticast_disable = virtio_dev_allmulticast_disable,
+ .mtu_set = virtio_mtu_set,
.dev_infos_get = virtio_dev_info_get,
.stats_get = virtio_dev_stats_get,
.xstats_get = virtio_dev_xstats_get,
--
2.9.3.windows.1
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2016-09-28 15:31 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-21 23:11 [PATCH v6] net/virtio: add set_mtu in virtio Dey
2016-09-21 23:22 ` Stephen Hemminger
2016-09-22 0:08 ` Dey, Souvik
2016-09-22 1:45 ` Stephen Hemminger
2016-09-22 2:29 ` Dey, Souvik
2016-09-23 7:53 ` Yuanhan Liu
2016-09-23 9:07 ` Kavanagh, Mark B
2016-09-23 15:17 ` Dey, Souvik
2016-09-26 3:21 ` Yuanhan Liu
2016-09-27 15:41 ` Dey, Souvik
2016-09-27 18:56 ` Stephen Hemminger
2016-09-27 20:44 ` Dey, Souvik
2016-09-27 23:11 ` Yuanhan Liu
2016-09-28 15:31 ` Dey, Souvik
2016-09-22 15:57 ` Stephen Hemminger
-- strict thread matches above, loose matches on Subject: below --
2016-09-21 21:10 Dey
2016-09-16 17:13 [PATCH v5]net/virtio: add mtu set " souvikdey33
2016-09-22 13:56 ` [PATCH v6] net/virtio: add set_mtu " Dey
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.