mbox series

[V1,0/2] examples/ethtool: fix MTU set and add MTU query

Message ID 1619693609-28244-1-git-send-email-humin29@huawei.com (mailing list archive)
Headers
Series examples/ethtool: fix MTU set and add MTU query |

Message

humin (Q) April 29, 2021, 10:53 a.m. UTC 
  From: Huisong Li <lihuisong@huawei.com>

This patchset fixes MTU data type when set MTU, and supports the query
of MTU.

Huisong Li (2):
  examples/ethtool: fix data type of MTU
  examples/ethtool: support the query of MTU

 examples/ethtool/ethtool-app/ethapp.c | 54 ++++++++++++++++++----------
 examples/ethtool/lib/rte_ethtool.c    | 16 ++++++++---
 examples/ethtool/lib/rte_ethtool.h    | 16 ++++++++++-
 3 files changed, 63 insertions(+), 23 deletions(-)

Comments

humin (Q) June 28, 2021, 3:23 a.m. UTC | #1 
Hi, all,
	any comments?

在 2021/4/29 18:53, Min Hu (Connor) 写道:
> From: Huisong Li <lihuisong@huawei.com>
> 
> This patchset fixes MTU data type when set MTU, and supports the query
> of MTU.
> 
> Huisong Li (2):
>    examples/ethtool: fix data type of MTU
>    examples/ethtool: support the query of MTU
> 
>   examples/ethtool/ethtool-app/ethapp.c | 54 ++++++++++++++++++----------
>   examples/ethtool/lib/rte_ethtool.c    | 16 ++++++++---
>   examples/ethtool/lib/rte_ethtool.h    | 16 ++++++++++-
>   3 files changed, 63 insertions(+), 23 deletions(-)
>
Stephen Hemminger July 3, 2023, 9:37 p.m. UTC | #2 
On Mon, 28 Jun 2021 11:23:12 +0800
"Min Hu (Connor)" <humin29@huawei.com> wrote:

> Hi, all,
> 	any comments?
> 
> 在 2021/4/29 18:53, Min Hu (Connor) 写道:
> > From: Huisong Li <lihuisong@huawei.com>
> > 
> > This patchset fixes MTU data type when set MTU, and supports the query
> > of MTU.
> > 
> > Huisong Li (2):
> >    examples/ethtool: fix data type of MTU
> >    examples/ethtool: support the query of MTU
> > 
> >   examples/ethtool/ethtool-app/ethapp.c | 54 ++++++++++++++++++----------
> >   examples/ethtool/lib/rte_ethtool.c    | 16 ++++++++---
> >   examples/ethtool/lib/rte_ethtool.h    | 16 ++++++++++-
> >   3 files changed, 63 insertions(+), 23 deletions(-)
> >   

There were several review comments.
In patch 1, the comment was that strtoul will return unsigned long.
Therefore new_mtu should be of type unsigned long, and check that is
is less than UINT16_MAX.

For the second patch, David's comment was that get and set should
logically be separate functions. The overlap was poor design in original
ethtool program, and lets not repeat that.

Overall, not many people looked at this patch because the ethtool
example is one of those "throw it over the wall and forget it" applications
that originally came from Cisco, and has seen little interest since then.