Re: [PATCH v2 1/5] kni: add API to set link status on kernel interface

From: Igor Ryzhov <iryzhov@nfware.com>
To: Dan Gora <dg@adax.com>
Cc: Ferruh Yigit <ferruh.yigit@intel.com>,
	dev@dpdk.org, Stephen Hemminger <stephen@networkplumber.org>
Subject: Re: [PATCH v2 1/5] kni: add API to set link status on kernel interface
Date: Fri, 28 Sep 2018 11:02:42 +0300	[thread overview]
Message-ID: <CAF+s_FztUxxoq75ZgrdYk_atqYBqCC-NJ45ZdDpvTn_+Zkegsw@mail.gmail.com> (raw)
In-Reply-To: <CAGyogRaNVQqgZLVsAHjH0Hwma6_Y+16CdjLpAkBg+Me3bL7xqw@mail.gmail.com>

Hi Dan, Ferruh,

Why do we need "struct rte_eth_link" as a parameter at all?
Only link status is used in the function – let's use it only:

rte_kni_update_link(struct rte_kni *kni, int link_status) /* 0 – down, 1 –
up */

It will also solve your differences as we won't have any "redundant"
information to print :)

Best regards,
Igor

On Fri, Sep 28, 2018 at 2:52 AM Dan Gora <dg@adax.com> wrote:

> On Thu, Sep 27, 2018 at 8:44 PM, Ferruh Yigit <ferruh.yigit@intel.com>
> wrote:
> >> Well, yes the link_status (link up, link down) _is_ applied to the KNI
> >> interface.  When that occurs, most people want to know what the link
> >> speed is that the link came up at.
> >
> > +1 to this, people would like to know link speed of the interface.
> > Are you printing link speed of interface? You are printing whatever user
> pass to
> > API.
>
> There is no such thing as "link speed of the interface".  The link
> speed is the speed of the underlying Ethernet link that the interface
> corresponds to.  This is true for all other ethernet interfaces in the
> kernel.
>
> > I guess you trust to user to provide correct values there, but since
> only link
> > up & down matters, what prevents user to leave other fields, like speed,
> just
> > random values?
>
> Nothing.  What prevents anyone from providing random values for
> anything?  The point of the API was to make it super simple, just:
>
> rte_eth_link_get_nowait(portid, &link);
> rte_kni_update_link(p[portid]->kni[i], &link);
>
> No messing around with the link info retrieved from
> rte_eth_link_get(_nowait), just dump in the struct that was returned.
>