From: "Dey, Souvik" <sodey@sonusnet.com>
To: Yuanhan Liu <yuanhan.liu@linux.intel.com>,
"Kavanagh, Mark B" <mark.b.kavanagh@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
"stephen@networkplumber.org" <stephen@networkplumber.org>
Subject: Re: [dpdk-dev] [PATCH v4]net/virtio: add mtu set in virtio
Date: Tue, 20 Sep 2016 18:42:02 +0000 [thread overview]
Message-ID: <BN3PR03MB1494B648BA0DD8496B6B1124DAF70@BN3PR03MB1494.namprd03.prod.outlook.com> (raw)
In-Reply-To: <20160920071152.GR23158@yliu-dev.sh.intel.com>
I have already taken care of this in v5 of the patch , If possible please review the same .
-----Original Message-----
From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
Sent: Tuesday, September 20, 2016 3:12 AM
To: Kavanagh, Mark B <mark.b.kavanagh@intel.com>
Cc: Dey, Souvik <sodey@sonusnet.com>; dev@dpdk.org; stephen@networkplumber.org
Subject: Re: [PATCH v4]net/virtio: add mtu set in virtio
On Wed, Sep 14, 2016 at 12:15:37PM +0000, Kavanagh, Mark B wrote:
> >
> >>+{
> >>+ struct rte_eth_dev_info dev_info;
> >>+ uint32_t ether_hdr_len = ETHER_HDR_LEN + ETHER_CRC_LEN +
> >>+VLAN_TAG_LEN;
> >>+ uint32_t frame_size = mtu + ether_hdr_len;
> >>+
> >>+ virtio_dev_info_get(dev, &dev_info);
> >>+
> >>+ if (mtu < dev_info.min_rx_bufsize || frame_size >
> >>+dev_info.max_rx_pktlen) {
> >
> >It's not clear to me whether 'mtu' in this case should be compared
> >with ETHER_MIN_MTU, as per other DPDK drivers, or alternatively
> >whether 'frame_size' should be compared with dev_info.min_rx_bufsize.
> >Any thoughts?
> >[Dey, Souvik] I am not sure why virtio min_rx_bufsize is less than
> >ETHER_MIN_MTU, i think it will be good to have the compare statement
> >as If(frame_size < ETHER_MIN_MTU || frame_size >
> >dev_info.max_rx_pktlen) , then error. What do you suggest ?
>
> Again, this all depends on what 'mtu' means in this context.
>
> Since you mentioned previously that it relates to the packet (i.e. L3) length, and not the frame (i.e. L2) length, I would suggest that the comparison should be:
>
> if (mtu < ETHER_MIN_MTU || frame_size > dev_info.max_rx_pktlen)
>
> Yuanhan, any thoughts on this?
I think you are right. At least, that's how the ixgbe PMD driver code looks like.
--yliu
next prev parent reply other threads:[~2016-09-20 18:42 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-07 4:18 souvikdey33
2016-09-07 4:21 ` Dey, Souvik
2016-09-09 7:00 ` Yuanhan Liu
2016-09-09 14:19 ` Dey, Souvik
2016-09-09 15:05 ` Kavanagh, Mark B
2016-09-09 15:33 ` Dey, Souvik
2016-09-09 15:42 ` Yuanhan Liu
2016-09-09 15:43 ` Kavanagh, Mark B
2016-09-09 18:16 ` Dey, Souvik
2016-09-12 14:25 ` Dey, Souvik
2016-09-12 14:47 ` Kavanagh, Mark B
2016-09-12 15:11 ` Dey, Souvik
2016-09-14 0:25 ` Dey, Souvik
2016-09-14 4:32 ` Yuanhan Liu
2016-09-14 12:15 ` Kavanagh, Mark B
2016-09-14 21:12 ` Dey, Souvik
2016-09-20 7:11 ` Yuanhan Liu
2016-09-20 18:42 ` Dey, Souvik [this message]
2016-09-21 2:21 ` Yuanhan Liu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=BN3PR03MB1494B648BA0DD8496B6B1124DAF70@BN3PR03MB1494.namprd03.prod.outlook.com \
--to=sodey@sonusnet.com \
--cc=dev@dpdk.org \
--cc=mark.b.kavanagh@intel.com \
--cc=stephen@networkplumber.org \
--cc=yuanhan.liu@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).