[v2,1/5] kni: add API to set link status on kernel interface
Checks
Commit Message
Add a new API function to KNI, rte_kni_update_link() to allow DPDK
applications to update the link status for KNI network interfaces in
the linux kernel.
Signed-off-by: Dan Gora <dg@adax.com>
---
lib/librte_kni/rte_kni.c | 57 ++++++++++++++++
lib/librte_kni/rte_kni.h | 18 ++++++
lib/librte_kni/rte_kni_version.map | 6 ++
test/test/test_kni.c | 100 ++++++++++++++++++++++++++++-
4 files changed, 180 insertions(+), 1 deletion(-)
Comments
On 9/19/2018 8:55 PM, Dan Gora wrote:
> Add a new API function to KNI, rte_kni_update_link() to allow DPDK
> applications to update the link status for KNI network interfaces in
> the linux kernel.
>
> Signed-off-by: Dan Gora <dg@adax.com>
<...>
> +int __rte_experimental
> +rte_kni_update_link(struct rte_kni *kni, struct rte_eth_link *link)
> +{
> + char path[64];
> + char carrier[2];
> + const char *new_carrier;
> + int fd, ret;
> +
> + if (kni == NULL || link == NULL)
> + return -1;
> +
> + snprintf(path, sizeof(path), "/sys/devices/virtual/net/%s/carrier",
> + kni->name);
> +
> + fd = open(path, O_RDWR);
> + if (fd == -1) {
> + RTE_LOG(ERR, KNI, "Failed to open file: %s.\n", path);
> + return -1;
> + }
> +
> + ret = read(fd, carrier, 2);
> + if (ret < 1) {
> + /* Cannot read carrier until interface is marked
> + * 'up', so don't log an error.
> + */
> + close(fd);
> + return -1;
> + }
> +
> + new_carrier = (link->link_status == ETH_LINK_UP) ? "1" : "0";
> + ret = write(fd, new_carrier, 1);
> + if (ret < 1) {
> + RTE_LOG(ERR, KNI, "Failed to write file: %s.\n", path);
> + close(fd);
> + return -1;
> + }
> +
> + if (strncmp(carrier, new_carrier, 1)) {
> + if (link->link_status == ETH_LINK_UP) {
> + RTE_LOG(INFO, KNI, "%s NIC Link is Up %d Mbps %s %s.\n",
> + kni->name,
> + link->link_speed,
> + link->link_autoneg ? "(AutoNeg)" : "(Fixed)",
> + link->link_duplex ?
> + "Full Duplex" : "Half Duplex");
These link values are coming from user and not reflected to the kni interface,
printing them here can cause a misunderstanding that they have been applied.
I think only link status should be printed here.
<...>
> @@ -148,9 +239,16 @@ test_kni_loop(__rte_unused void *arg)
> ret = -1;
> if (system(IFCONFIG TEST_KNI_PORT" mtu 1400") == -1)
> ret = -1;
> +
> + ret = kni_change_link();
> + if (ret != 0) {
> + test_kni_processing_flag = 1;
> + return ret;
> + }
I thinks this is wrong place to call kni_change_link(), this is test_kni_loop()
created by test_kni_processing() that does packet processing tests.
I think better to call directly from test_kni(), perhaps before
test_kni_processing() call?
On Wed, Sep 26, 2018 at 10:59 AM, Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> On 9/19/2018 8:55 PM, Dan Gora wrote:
>> Add a new API function to KNI, rte_kni_update_link() to allow DPDK
>> applications to update the link status for KNI network interfaces in
>> the linux kernel.
>>
>> Signed-off-by: Dan Gora <dg@adax.com>
>
> <...>
>
>> +int __rte_experimental
>> +rte_kni_update_link(struct rte_kni *kni, struct rte_eth_link *link)
>> +{
>> + char path[64];
>> + char carrier[2];
>> + const char *new_carrier;
>> + int fd, ret;
>> +
>> + if (kni == NULL || link == NULL)
>> + return -1;
>> +
>> + snprintf(path, sizeof(path), "/sys/devices/virtual/net/%s/carrier",
>> + kni->name);
>> +
>> + fd = open(path, O_RDWR);
>> + if (fd == -1) {
>> + RTE_LOG(ERR, KNI, "Failed to open file: %s.\n", path);
>> + return -1;
>> + }
>> +
>> + ret = read(fd, carrier, 2);
>> + if (ret < 1) {
>> + /* Cannot read carrier until interface is marked
>> + * 'up', so don't log an error.
>> + */
>> + close(fd);
>> + return -1;
>> + }
>> +
>> + new_carrier = (link->link_status == ETH_LINK_UP) ? "1" : "0";
>> + ret = write(fd, new_carrier, 1);
>> + if (ret < 1) {
>> + RTE_LOG(ERR, KNI, "Failed to write file: %s.\n", path);
>> + close(fd);
>> + return -1;
>> + }
>> +
>> + if (strncmp(carrier, new_carrier, 1)) {
>> + if (link->link_status == ETH_LINK_UP) {
>> + RTE_LOG(INFO, KNI, "%s NIC Link is Up %d Mbps %s %s.\n",
>> + kni->name,
>> + link->link_speed,
>> + link->link_autoneg ? "(AutoNeg)" : "(Fixed)",
>> + link->link_duplex ?
>> + "Full Duplex" : "Half Duplex");
>
> These link values are coming from user and not reflected to the kni interface,
> printing them here can cause a misunderstanding that they have been applied.
> I think only link status should be printed here.
>
There is nothing to "reflect" to the kernel interface, nor to apply to
the kernel interface. This is exactly how every other kernel driver
works on link status changes. There is no "netif_set_speed()'
function. When a link status change occurs the kernel driver calls
netif_carrier_on/off() and prints a message like this one.
> <...>
>
>> @@ -148,9 +239,16 @@ test_kni_loop(__rte_unused void *arg)
>> ret = -1;
>> if (system(IFCONFIG TEST_KNI_PORT" mtu 1400") == -1)
>> ret = -1;
>> +
>> + ret = kni_change_link();
>> + if (ret != 0) {
>> + test_kni_processing_flag = 1;
>> + return ret;
>> + }
>
> I thinks this is wrong place to call kni_change_link(), this is test_kni_loop()
> created by test_kni_processing() that does packet processing tests.
>
Well, no it's not "wrong". The interface has to be up to change the
link state, so this is a convenient place to do it.
> I think better to call directly from test_kni(), perhaps before
> test_kni_processing() call?
Because then we'd have to add code to set the interface up and down
again, and there really is no point. This is as good a place as any.
It does not affect the data transfer tests at all.
-d
On 9/26/2018 3:55 PM, Dan Gora wrote:
> On Wed, Sep 26, 2018 at 10:59 AM, Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>> On 9/19/2018 8:55 PM, Dan Gora wrote:
>>> Add a new API function to KNI, rte_kni_update_link() to allow DPDK
>>> applications to update the link status for KNI network interfaces in
>>> the linux kernel.
>>>
>>> Signed-off-by: Dan Gora <dg@adax.com>
>>
>> <...>
>>
>>> +int __rte_experimental
>>> +rte_kni_update_link(struct rte_kni *kni, struct rte_eth_link *link)
>>> +{
>>> + char path[64];
>>> + char carrier[2];
>>> + const char *new_carrier;
>>> + int fd, ret;
>>> +
>>> + if (kni == NULL || link == NULL)
>>> + return -1;
>>> +
>>> + snprintf(path, sizeof(path), "/sys/devices/virtual/net/%s/carrier",
>>> + kni->name);
>>> +
>>> + fd = open(path, O_RDWR);
>>> + if (fd == -1) {
>>> + RTE_LOG(ERR, KNI, "Failed to open file: %s.\n", path);
>>> + return -1;
>>> + }
>>> +
>>> + ret = read(fd, carrier, 2);
>>> + if (ret < 1) {
>>> + /* Cannot read carrier until interface is marked
>>> + * 'up', so don't log an error.
>>> + */
>>> + close(fd);
>>> + return -1;
>>> + }
>>> +
>>> + new_carrier = (link->link_status == ETH_LINK_UP) ? "1" : "0";
>>> + ret = write(fd, new_carrier, 1);
>>> + if (ret < 1) {
>>> + RTE_LOG(ERR, KNI, "Failed to write file: %s.\n", path);
>>> + close(fd);
>>> + return -1;
>>> + }
>>> +
>>> + if (strncmp(carrier, new_carrier, 1)) {
>>> + if (link->link_status == ETH_LINK_UP) {
>>> + RTE_LOG(INFO, KNI, "%s NIC Link is Up %d Mbps %s %s.\n",
>>> + kni->name,
>>> + link->link_speed,
>>> + link->link_autoneg ? "(AutoNeg)" : "(Fixed)",
>>> + link->link_duplex ?
>>> + "Full Duplex" : "Half Duplex");
>>
>> These link values are coming from user and not reflected to the kni interface,
>> printing them here can cause a misunderstanding that they have been applied.
>> I think only link status should be printed here.
>>
>
> There is nothing to "reflect" to the kernel interface, nor to apply to
> the kernel interface. This is exactly how every other kernel driver
> works on link status changes. There is no "netif_set_speed()'
> function. When a link status change occurs the kernel driver calls
> netif_carrier_on/off() and prints a message like this one.
I am not suggesting reflecting these into interface, I am just saying why do you
print them?
For example, "link->link_speed" this is coming as parameter to API, this API
does nothing with this value, why print is here?
I assume you are using "rte_eth_link" as parameter, instead of basic
"link_status" to make it extendible in the feature. If so please print those
other values when function extended, right now only link_status matters.
>
>> <...>
>>
>>> @@ -148,9 +239,16 @@ test_kni_loop(__rte_unused void *arg)
>>> ret = -1;
>>> if (system(IFCONFIG TEST_KNI_PORT" mtu 1400") == -1)
>>> ret = -1;
>>> +
>>> + ret = kni_change_link();
>>> + if (ret != 0) {
>>> + test_kni_processing_flag = 1;
>>> + return ret;
>>> + }
>>
>> I thinks this is wrong place to call kni_change_link(), this is test_kni_loop()
>> created by test_kni_processing() that does packet processing tests.
>>
>
> Well, no it's not "wrong". The interface has to be up to change the
> link state, so this is a convenient place to do it.
Are we agree that this function in unit test is to test packet processing part
of KNI?
And for example please check following coming patch:
https://patches.dpdk.org/patch/44730/
I think you want to benefit from "system(IFCONFIG TEST_KNI_PORT" up")" since
interface needs to be up to set the link, but you can do same call, not have to
use that one.
>
>> I think better to call directly from test_kni(), perhaps before
>> test_kni_processing() call?
>
> Because then we'd have to add code to set the interface up and down
> again, and there really is no point. This is as good a place as any.
> It does not affect the data transfer tests at all.
Doesn't affect the data transfer, agreed, but why confusing data transfer test
with link up/down calls? Why not have your function and set interface up and
down again?
>
> -d
>
On Wed, Sep 26, 2018 at 1:42 PM, Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>> There is nothing to "reflect" to the kernel interface, nor to apply to
>> the kernel interface. This is exactly how every other kernel driver
>> works on link status changes. There is no "netif_set_speed()'
>> function. When a link status change occurs the kernel driver calls
>> netif_carrier_on/off() and prints a message like this one.
>
> I am not suggesting reflecting these into interface, I am just saying why do you
> print them?
Because the information is useful and because every other Ethernet
driver does the same thing when the link status changes.
See the Linux kernel:
ixgbe_watchdog_link_is_up(): drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
print_link_info(): drivers/net/ethernet/cavium/liquidio/lio_main.c
bnx2_report_link(): drivers/net/ethernet/broadcom/bnx2.c
and dozens of other examples.
Imagine you plug your 1G ethernet cable into a 10/100Mbps hub. Yes
the link will be up, but it's the wrong speed.
Imagine you had accidentally forced your 1G connection to 10Mbps
fixed. It will only come up at 10Mbps. Wouldn't you like to know
that autoNeg was disabled?
> For example, "link->link_speed" this is coming as parameter to API, this API
> does nothing with this value, why print is here?
Because wouldn't you like to know if your link has connected at the
correct speed? I would.
> I assume you are using "rte_eth_link" as parameter, instead of basic
> "link_status" to make it extendible in the feature. If so please print those
> other values when function extended, right now only link_status matters.
No, all that information matters. If you have autoNeg disabled or are
connecting to a broken piece of equipment, the link can come up but be
at the wrong speed.
There is not much to extend this function with. The only action we
can take, other than to print the information, is to write to the /sys
file to have the KNI kernel module call netif_carrier_on/off() for us.
>>>> @@ -148,9 +239,16 @@ test_kni_loop(__rte_unused void *arg)
>>>> ret = -1;
>>>> if (system(IFCONFIG TEST_KNI_PORT" mtu 1400") == -1)
>>>> ret = -1;
>>>> +
>>>> + ret = kni_change_link();
>>>> + if (ret != 0) {
>>>> + test_kni_processing_flag = 1;
>>>> + return ret;
>>>> + }
>>>
>>> I thinks this is wrong place to call kni_change_link(), this is test_kni_loop()
>>> created by test_kni_processing() that does packet processing tests.
>>>
>>
>> Well, no it's not "wrong". The interface has to be up to change the
>> link state, so this is a convenient place to do it.
>
> Are we agree that this function in unit test is to test packet processing part
> of KNI?
> And for example please check following coming patch:
> https://patches.dpdk.org/patch/44730/
>
> I think you want to benefit from "system(IFCONFIG TEST_KNI_PORT" up")" since
> interface needs to be up to set the link, but you can do same call, not have to
> use that one.
ok, I'll fix this. Should I rebase the changes on top of this patch
(https://patches.dpdk.org/patch/44730/)? Is that patch going to get
accepted soon?
>> Because then we'd have to add code to set the interface up and down
>> again, and there really is no point. This is as good a place as any.
>> It does not affect the data transfer tests at all.
>
> Doesn't affect the data transfer, agreed, but why confusing data transfer test
> with link up/down calls? Why not have your function and set interface up and
> down again?
ok. I was just trying to minimize the number of changes. I'll send a new patch.
On 9/26/2018 7:56 PM, Dan Gora wrote:
> On Wed, Sep 26, 2018 at 1:42 PM, Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>>> There is nothing to "reflect" to the kernel interface, nor to apply to
>>> the kernel interface. This is exactly how every other kernel driver
>>> works on link status changes. There is no "netif_set_speed()'
>>> function. When a link status change occurs the kernel driver calls
>>> netif_carrier_on/off() and prints a message like this one.
>>
>> I am not suggesting reflecting these into interface, I am just saying why do you
>> print them?
>
> Because the information is useful and because every other Ethernet
> driver does the same thing when the link status changes.
It would be useful if it writes the values of virtual interface, but this API
prints user input.
The virtual interface may have different value, this API doesn't change anything
related other than link status, so why print user provided value.
Won't you think it will be confusing if the virtual interface values are
different than what printed.
Or won't user will think API changed those values to printed one in interface?
>
> See the Linux kernel:
>
> ixgbe_watchdog_link_is_up(): drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> print_link_info(): drivers/net/ethernet/cavium/liquidio/lio_main.c
> bnx2_report_link(): drivers/net/ethernet/broadcom/bnx2.c
>
> and dozens of other examples.
>
> Imagine you plug your 1G ethernet cable into a 10/100Mbps hub. Yes
> the link will be up, but it's the wrong speed.
> Imagine you had accidentally forced your 1G connection to 10Mbps
> fixed. It will only come up at 10Mbps. Wouldn't you like to know
> that autoNeg was disabled?
>
>> For example, "link->link_speed" this is coming as parameter to API, this API
>> does nothing with this value, why print is here?
>
> Because wouldn't you like to know if your link has connected at the
> correct speed? I would.
>
>> I assume you are using "rte_eth_link" as parameter, instead of basic
>> "link_status" to make it extendible in the feature. If so please print those
>> other values when function extended, right now only link_status matters.
>
> No, all that information matters. If you have autoNeg disabled or are
> connecting to a broken piece of equipment, the link can come up but be
> at the wrong speed.
>
> There is not much to extend this function with. The only action we
> can take, other than to print the information, is to write to the /sys
> file to have the KNI kernel module call netif_carrier_on/off() for us.
>
>>>>> @@ -148,9 +239,16 @@ test_kni_loop(__rte_unused void *arg)
>>>>> ret = -1;
>>>>> if (system(IFCONFIG TEST_KNI_PORT" mtu 1400") == -1)
>>>>> ret = -1;
>>>>> +
>>>>> + ret = kni_change_link();
>>>>> + if (ret != 0) {
>>>>> + test_kni_processing_flag = 1;
>>>>> + return ret;
>>>>> + }
>>>>
>>>> I thinks this is wrong place to call kni_change_link(), this is test_kni_loop()
>>>> created by test_kni_processing() that does packet processing tests.
>>>>
>>>
>>> Well, no it's not "wrong". The interface has to be up to change the
>>> link state, so this is a convenient place to do it.
>>
>> Are we agree that this function in unit test is to test packet processing part
>> of KNI?
>> And for example please check following coming patch:
>> https://patches.dpdk.org/patch/44730/
>>
>> I think you want to benefit from "system(IFCONFIG TEST_KNI_PORT" up")" since
>> interface needs to be up to set the link, but you can do same call, not have to
>> use that one.
>
> ok, I'll fix this. Should I rebase the changes on top of this patch
> (https://patches.dpdk.org/patch/44730/)? Is that patch going to get
> accepted soon?
>
>>> Because then we'd have to add code to set the interface up and down
>>> again, and there really is no point. This is as good a place as any.
>>> It does not affect the data transfer tests at all.
>>
>> Doesn't affect the data transfer, agreed, but why confusing data transfer test
>> with link up/down calls? Why not have your function and set interface up and
>> down again?
>
> ok. I was just trying to minimize the number of changes. I'll send a new patch.
>
On Thu, Sep 27, 2018 at 8:35 AM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>
> On 9/26/2018 7:56 PM, Dan Gora wrote:
> > On Wed, Sep 26, 2018 at 1:42 PM, Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> >>> There is nothing to "reflect" to the kernel interface, nor to apply to
> >>> the kernel interface. This is exactly how every other kernel driver
> >>> works on link status changes. There is no "netif_set_speed()'
> >>> function. When a link status change occurs the kernel driver calls
> >>> netif_carrier_on/off() and prints a message like this one.
> >>
> >> I am not suggesting reflecting these into interface, I am just saying why do you
> >> print them?
> >
> > Because the information is useful and because every other Ethernet
> > driver does the same thing when the link status changes.
>
> It would be useful if it writes the values of virtual interface, but this API
> prints user input.
>
I'm sorry, Ferruh, I really don't understand what you are referring to
here. What virtual interface are you talking about?
> The virtual interface may have different value, this API doesn't change anything
> related other than link status, so why print user provided value.
Again, because all this information is useful if the KNI interface is
reflecting the state of a real physical ethenet interface. If the KNI
interface is not reflecting a real peice of hardware, the user can
continue not using this new API function or do something like set the
speed to 0. This is what we get from ENA PMD drivers on amazon for
instance. It's perfectly reasonable.
>
> Won't you think it will be confusing if the virtual interface values are
> different than what printed.
If what values are different from what? I'm really confused as to
what use case you are referring to here. Can you give a concrete
example?
> Or won't user will think API changed those values to printed one in interface?
again, what values? Changed how?
On 9/27/2018 4:40 PM, Dan Gora wrote:
> On Thu, Sep 27, 2018 at 8:35 AM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>>
>> On 9/26/2018 7:56 PM, Dan Gora wrote:
>>> On Wed, Sep 26, 2018 at 1:42 PM, Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>>>>> There is nothing to "reflect" to the kernel interface, nor to apply to
>>>>> the kernel interface. This is exactly how every other kernel driver
>>>>> works on link status changes. There is no "netif_set_speed()'
>>>>> function. When a link status change occurs the kernel driver calls
>>>>> netif_carrier_on/off() and prints a message like this one.
>>>>
>>>> I am not suggesting reflecting these into interface, I am just saying why do you
>>>> print them?
>>>
>>> Because the information is useful and because every other Ethernet
>>> driver does the same thing when the link status changes.
>>
>> It would be useful if it writes the values of virtual interface, but this API
>> prints user input.
>>
> I'm sorry, Ferruh, I really don't understand what you are referring to
> here. What virtual interface are you talking about?
Virtual interface is KNI interface, Linux virtual network interface.
Let me try again,
You are adding a new API to KNI library, which is to set link status of KNI
interface.
This API prints some link related values in its log. But these values are not
applied to KNI interface or not the values coming from KNI interface. These are
just values provided by user to the API, I am saying printing these values in
log can be confusing, the user may think API applies these values to KNI interface.
>
>> The virtual interface may have different value, this API doesn't change anything
>> related other than link status, so why print user provided value.
>
> Again, because all this information is useful if the KNI interface is
> reflecting the state of a real physical ethenet interface. If the KNI
> interface is not reflecting a real peice of hardware, the user can
> continue not using this new API function or do something like set the
> speed to 0. This is what we get from ENA PMD drivers on amazon for
> instance. It's perfectly reasonable.
>
>>
>> Won't you think it will be confusing if the virtual interface values are
>> different than what printed.
>
> If what values are different from what? I'm really confused as to
> what use case you are referring to here. Can you give a concrete
> example?
>
>> Or won't user will think API changed those values to printed one in interface?
>
> again, what values? Changed how?
>
On Thu, Sep 27, 2018 at 6:49 PM, Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>>> It would be useful if it writes the values of virtual interface, but this API
>>> prints user input.
>>>
>> I'm sorry, Ferruh, I really don't understand what you are referring to
>> here. What virtual interface are you talking about?
>
> Virtual interface is KNI interface, Linux virtual network interface.
>
>
> Let me try again,
>
> You are adding a new API to KNI library, which is to set link status of KNI
> interface.
>
> This API prints some link related values in its log. But these values are not
> applied to KNI interface or not the values coming from KNI interface. These are
> just values provided by user to the API, I am saying printing these values in
> log can be confusing, the user may think API applies these values to KNI interface.
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. Again, this is how every other
Ethernet driver in linux works. I doubt that someone would be
confused by that.
If the KNI interface is purely virtual (that is, it does not
correspond to any physical Ethernet port), then the DPDK application
writer is under no obligation to use this API function, or they can
use it and just set the speed to 0. If the end user understands that
the KNI interface is purely virtual, then they should not be confused
that the speed is 0 or that the duplex/autoNeg values are irrelevant.
I just don't see how this could cause any confusion.
On 9/28/2018 12:05 AM, Dan Gora wrote:
> On Thu, Sep 27, 2018 at 6:49 PM, Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>>>> It would be useful if it writes the values of virtual interface, but this API
>>>> prints user input.
>>>>
>>> I'm sorry, Ferruh, I really don't understand what you are referring to
>>> here. What virtual interface are you talking about?
>>
>> Virtual interface is KNI interface, Linux virtual network interface.
>>
>>
>> Let me try again,
>>
>> You are adding a new API to KNI library, which is to set link status of KNI
>> interface.
>>
>> This API prints some link related values in its log. But these values are not
>> applied to KNI interface or not the values coming from KNI interface. These are
>> just values provided by user to the API, I am saying printing these values in
>> log can be confusing, the user may think API applies these values to KNI interface.
>
> 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.
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?
> Again, this is how every other
> Ethernet driver in linux works. I doubt that someone would be
> confused by that.
>
> If the KNI interface is purely virtual (that is, it does not
> correspond to any physical Ethernet port), then the DPDK application
> writer is under no obligation to use this API function, or they can
> use it and just set the speed to 0. If the end user understands that
> the KNI interface is purely virtual, then they should not be confused
> that the speed is 0 or that the duplex/autoNeg values are irrelevant.
>
> I just don't see how this could cause any confusion.
>
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.
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.
>
On 9/28/2018 12:51 AM, Dan Gora 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.
This is an API to set link status of KNI interface, KNI interface is a virtual
interface and no need to be backed by a physical interface at all.
Only kni sample application uses it in a way to match a physical interface to a
KNI interface, but please check KNI PMD where you can have multiple KNI
interface without any physical device at all.
>
>> 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);
You are only thinking about this use case. Normally the input to API should be
verified right, for this case there is no way to verify it and vales are not
used at all, it is just printed in API.
OK to print link information but please do so in sample application before
calling API, not in API please.
>
> No messing around with the link info retrieved from
> rte_eth_link_get(_nowait), just dump in the struct that was returned.
>
On Fri, Sep 28, 2018 at 5:03 AM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>
> On 9/28/2018 12:51 AM, Dan Gora 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.
>
> This is an API to set link status of KNI interface, KNI interface is a virtual
> interface and no need to be backed by a physical interface at all.
Yes, I understand that.
> Only kni sample application uses it in a way to match a physical interface to a
> KNI interface, but please check KNI PMD where you can have multiple KNI
> interface without any physical device at all.
Yes, I understand that.. As I pointed out previously, if there is no
physical device which corresponds to this KNI interface, the
application can:
1) Not use this API at all, just as they do now.
2) Use the API and set the state to linkup/0Mbps/autoNeg/Full. This
is perfectly reasonable and no one would be confused by this.
> >> 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);
>
> You are only thinking about this use case. Normally the input to API should be
> verified right, for this case there is no way to verify it and vales are not
> used at all, it is just printed in API.
In most applications it is useful to have a message printed which
shows the *change* in link status as well as the speed that the link
came up at. If you don't provide the link speed, etc information,
then the API is not really useful. Yes the application writer can
provide whatever they want for the link speed/duplex/autoneg, but so
what? Their application will have a nonsensical log message. It's
not DPDK's job to enforce sanity on everyone's application.
And no, not every input to every API function is verified. There are
lots of examples in the DPDK where this is not the case. The
parameters should be verified to ensure that they do not cause the
application to "break", not necessarily so that they make sense
(whatever that would mean in this case).
It was recommended that we change the rte_kni_update_link() API to
only use the link status as an input and print the log messages
outside the API in the application code. However think about what
that would entail:
You only want to log *changes* in the link state, not log a message
when you set the same state. This means that either:
1) the application would have to store the previous carrier state.
2) rte_kni_update_link could return the previous value read from the
/sys/.../carrier file.
3) the application could read the /sys/.../carrier file before calling
rte_kni_update_link to read the previous carrier state.
In case 1 you're obliging all users of this API to carry around this
extra state for no real reason. The API can easily read the
/sys/.../carrier file to see the previous state while it has it open,
just as it does in this patch.
In case 2, the application can easily detect when the state changes,
but then you end up with a return value something like -1 for errors,
0 for previous_link == down, 1 for previous_link == up. I fail to see
how this is a better API design that what has already been proposed,
but it's at least somewhat useful, even if awkward.
In the DPDK application, you get something like:
rte_eth_link_get_nowait(portid, &link);
prev_link = rte_kni_update_link(p[portid]->kni[i], link.link_status);
if (prev_link >= 0)
{
if (prev_link == 0 && link.link_status == 1)
printf("LINKUP: speed %d duplex %s autoneg %s\n", ....)
else if (prev_link == 1 && link.link_status == 0)
printf("LINKDOWN:\n");
} else {
printf("Error: something went wrong...\n");
}
I don't really see how this is better than:
rte_eth_link_get_nowait(portid, &link);
rte_kni_update_link(p[portid]->kni[i], &link);
but that's my opinion...
In case 3, you might as well not even have this API since you're
duplicating half of the same code.
I would be willing to implement case 2, however I still think that it
is not as good a design and will unnecessarily complicate application
code compared to the current patch here. Cases 1 and 3 are
non-starters for me. Our application needs log messages only when the
link *changes* state. These log messages need to include the link
speed/duplex/autoneg info. I think that most, if not all,
applications would want that too. It's how every other physical
ethernet driver in the linux kernel works. DPDK applications which
use only KNI virtual interfaces without a physical interface are under
no obligation to use this API function.
thanks
dan
On 10/3/2018 8:07 PM, Dan Gora wrote:
> On Fri, Sep 28, 2018 at 5:03 AM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>>
>> On 9/28/2018 12:51 AM, Dan Gora 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.
>>
>> This is an API to set link status of KNI interface, KNI interface is a virtual
>> interface and no need to be backed by a physical interface at all.
>
> Yes, I understand that.
>
>> Only kni sample application uses it in a way to match a physical interface to a
>> KNI interface, but please check KNI PMD where you can have multiple KNI
>> interface without any physical device at all.
>
> Yes, I understand that.. As I pointed out previously, if there is no
> physical device which corresponds to this KNI interface, the
> application can:
>
> 1) Not use this API at all, just as they do now.
This API has nothing with if KNI interface has corresponding physical device or
not, all KNI users can use it.
> 2) Use the API and set the state to linkup/0Mbps/autoNeg/Full. This
> is perfectly reasonable and no one would be confused by this.
That is the think, you are not setting anything, you are just printing
"0Mbps/autoNeg/Full" and assuming/hoping the _if_ there is a corresponding
physical device app sends the values of that device to API for printing.
Overall I am not sure if there is a value to discuss this more, and I don't want
this relatively small issue to block the patch, I will comment more to latest
version.
>
>>>> 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);
>>
>> You are only thinking about this use case. Normally the input to API should be
>> verified right, for this case there is no way to verify it and vales are not
>> used at all, it is just printed in API.
>
> In most applications it is useful to have a message printed which
> shows the *change* in link status as well as the speed that the link
> came up at. If you don't provide the link speed, etc information,
> then the API is not really useful. Yes the application writer can
> provide whatever they want for the link speed/duplex/autoneg, but so
> what? Their application will have a nonsensical log message. It's
> not DPDK's job to enforce sanity on everyone's application.
>
> And no, not every input to every API function is verified. There are
> lots of examples in the DPDK where this is not the case. The
> parameters should be verified to ensure that they do not cause the
> application to "break", not necessarily so that they make sense
> (whatever that would mean in this case).
>
> It was recommended that we change the rte_kni_update_link() API to
> only use the link status as an input and print the log messages
> outside the API in the application code. However think about what
> that would entail:
>
> You only want to log *changes* in the link state, not log a message
> when you set the same state. This means that either:
>
> 1) the application would have to store the previous carrier state.
> 2) rte_kni_update_link could return the previous value read from the
> /sys/.../carrier file.
> 3) the application could read the /sys/.../carrier file before calling
> rte_kni_update_link to read the previous carrier state.
>
> In case 1 you're obliging all users of this API to carry around this
> extra state for no real reason. The API can easily read the
> /sys/.../carrier file to see the previous state while it has it open,
> just as it does in this patch.
>
> In case 2, the application can easily detect when the state changes,
> but then you end up with a return value something like -1 for errors,
> 0 for previous_link == down, 1 for previous_link == up. I fail to see
> how this is a better API design that what has already been proposed,
> but it's at least somewhat useful, even if awkward.
>
> In the DPDK application, you get something like:
>
> rte_eth_link_get_nowait(portid, &link);
> prev_link = rte_kni_update_link(p[portid]->kni[i], link.link_status);
> if (prev_link >= 0)
> {
> if (prev_link == 0 && link.link_status == 1)
> printf("LINKUP: speed %d duplex %s autoneg %s\n", ....)
> else if (prev_link == 1 && link.link_status == 0)
> printf("LINKDOWN:\n");
> } else {
> printf("Error: something went wrong...\n");
> }
>
> I don't really see how this is better than:
>
> rte_eth_link_get_nowait(portid, &link);
> rte_kni_update_link(p[portid]->kni[i], &link);
>
> but that's my opinion...
>
> In case 3, you might as well not even have this API since you're
> duplicating half of the same code.
>
> I would be willing to implement case 2, however I still think that it
> is not as good a design and will unnecessarily complicate application
> code compared to the current patch here. Cases 1 and 3 are
> non-starters for me. Our application needs log messages only when the
> link *changes* state. These log messages need to include the link
> speed/duplex/autoneg info. I think that most, if not all,
> applications would want that too. It's how every other physical
> ethernet driver in the linux kernel works. DPDK applications which
> use only KNI virtual interfaces without a physical interface are under
> no obligation to use this API function.
>
> thanks
> dan
>
On Wed, Oct 10, 2018 at 11:09 AM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> > Yes, I understand that.. As I pointed out previously, if there is no
> > physical device which corresponds to this KNI interface, the
> > application can:
> >
> > 1) Not use this API at all, just as they do now.
>
> This API has nothing with if KNI interface has corresponding physical device or
> not, all KNI users can use it.
>
I don't know what this means.
> > 2) Use the API and set the state to linkup/0Mbps/autoNeg/Full. This
> > is perfectly reasonable and no one would be confused by this.
>
> That is the think, you are not setting anything, you are just printing
> "0Mbps/autoNeg/Full" and assuming/hoping the _if_ there is a corresponding
> physical device app sends the values of that device to API for printing.
ugh.. Again the API doesn't care if the values are "right". The
application writer probably does though.
> Overall I am not sure if there is a value to discuss this more, and I don't want
> this relatively small issue to block the patch, I will comment more to latest
> version.
Agreed, I don't know how else I can express myself to get my point across.
Can you at least comment on the rest of this email below that I spent
a lot of time writing, trying to explain what I'm talking about?
> >>>> 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);
> >>
> >> You are only thinking about this use case. Normally the input to API should be
> >> verified right, for this case there is no way to verify it and vales are not
> >> used at all, it is just printed in API.
> >
> > In most applications it is useful to have a message printed which
> > shows the *change* in link status as well as the speed that the link
> > came up at. If you don't provide the link speed, etc information,
> > then the API is not really useful. Yes the application writer can
> > provide whatever they want for the link speed/duplex/autoneg, but so
> > what? Their application will have a nonsensical log message. It's
> > not DPDK's job to enforce sanity on everyone's application.
> >
> > And no, not every input to every API function is verified. There are
> > lots of examples in the DPDK where this is not the case. The
> > parameters should be verified to ensure that they do not cause the
> > application to "break", not necessarily so that they make sense
> > (whatever that would mean in this case).
> >
> > It was recommended that we change the rte_kni_update_link() API to
> > only use the link status as an input and print the log messages
> > outside the API in the application code. However think about what
> > that would entail:
> >
> > You only want to log *changes* in the link state, not log a message
> > when you set the same state. This means that either:
> >
> > 1) the application would have to store the previous carrier state.
> > 2) rte_kni_update_link could return the previous value read from the
> > /sys/.../carrier file.
> > 3) the application could read the /sys/.../carrier file before calling
> > rte_kni_update_link to read the previous carrier state.
> >
> > In case 1 you're obliging all users of this API to carry around this
> > extra state for no real reason. The API can easily read the
> > /sys/.../carrier file to see the previous state while it has it open,
> > just as it does in this patch.
> >
> > In case 2, the application can easily detect when the state changes,
> > but then you end up with a return value something like -1 for errors,
> > 0 for previous_link == down, 1 for previous_link == up. I fail to see
> > how this is a better API design that what has already been proposed,
> > but it's at least somewhat useful, even if awkward.
> >
> > In the DPDK application, you get something like:
> >
> > rte_eth_link_get_nowait(portid, &link);
> > prev_link = rte_kni_update_link(p[portid]->kni[i], link.link_status);
> > if (prev_link >= 0)
> > {
> > if (prev_link == 0 && link.link_status == 1)
> > printf("LINKUP: speed %d duplex %s autoneg %s\n", ....)
> > else if (prev_link == 1 && link.link_status == 0)
> > printf("LINKDOWN:\n");
> > } else {
> > printf("Error: something went wrong...\n");
> > }
> >
> > I don't really see how this is better than:
> >
> > rte_eth_link_get_nowait(portid, &link);
> > rte_kni_update_link(p[portid]->kni[i], &link);
> >
> > but that's my opinion...
> >
> > In case 3, you might as well not even have this API since you're
> > duplicating half of the same code.
> >
> > I would be willing to implement case 2, however I still think that it
> > is not as good a design and will unnecessarily complicate application
> > code compared to the current patch here. Cases 1 and 3 are
> > non-starters for me. Our application needs log messages only when the
> > link *changes* state. These log messages need to include the link
> > speed/duplex/autoneg info. I think that most, if not all,
> > applications would want that too. It's how every other physical
> > ethernet driver in the linux kernel works. DPDK applications which
> > use only KNI virtual interfaces without a physical interface are under
> > no obligation to use this API function.
> >
> > thanks
> > dan
@@ -790,6 +790,63 @@ rte_kni_unregister_handlers(struct rte_kni *kni)
return 0;
}
+
+int __rte_experimental
+rte_kni_update_link(struct rte_kni *kni, struct rte_eth_link *link)
+{
+ char path[64];
+ char carrier[2];
+ const char *new_carrier;
+ int fd, ret;
+
+ if (kni == NULL || link == NULL)
+ return -1;
+
+ snprintf(path, sizeof(path), "/sys/devices/virtual/net/%s/carrier",
+ kni->name);
+
+ fd = open(path, O_RDWR);
+ if (fd == -1) {
+ RTE_LOG(ERR, KNI, "Failed to open file: %s.\n", path);
+ return -1;
+ }
+
+ ret = read(fd, carrier, 2);
+ if (ret < 1) {
+ /* Cannot read carrier until interface is marked
+ * 'up', so don't log an error.
+ */
+ close(fd);
+ return -1;
+ }
+
+ new_carrier = (link->link_status == ETH_LINK_UP) ? "1" : "0";
+ ret = write(fd, new_carrier, 1);
+ if (ret < 1) {
+ RTE_LOG(ERR, KNI, "Failed to write file: %s.\n", path);
+ close(fd);
+ return -1;
+ }
+
+ if (strncmp(carrier, new_carrier, 1)) {
+ if (link->link_status == ETH_LINK_UP) {
+ RTE_LOG(INFO, KNI, "%s NIC Link is Up %d Mbps %s %s.\n",
+ kni->name,
+ link->link_speed,
+ link->link_autoneg ? "(AutoNeg)" : "(Fixed)",
+ link->link_duplex ?
+ "Full Duplex" : "Half Duplex");
+ } else {
+ RTE_LOG(INFO, KNI, "%s NIC Link is Down.\n",
+ kni->name);
+ }
+ }
+
+ close(fd);
+
+ return 0;
+}
+
void
rte_kni_close(void)
{
@@ -21,6 +21,7 @@
#include <rte_memory.h>
#include <rte_mempool.h>
#include <rte_ether.h>
+#include <rte_ethdev.h>
#include <exec-env/rte_kni_common.h>
@@ -228,6 +229,23 @@ int rte_kni_register_handlers(struct rte_kni *kni, struct rte_kni_ops *ops);
*/
int rte_kni_unregister_handlers(struct rte_kni *kni);
+/**
+ * Update link status info for KNI port.
+ *
+ * Update the linkup/linkdown status of a KNI interface in the kernel.
+ *
+ * @param kni
+ * pointer to struct rte_kni.
+ * @param link
+ * pointer to struct rte_eth_link containing new interface status.
+ *
+ * @return
+ * On success: 0
+ * On failure: -1
+ */
+int __rte_experimental
+rte_kni_update_link(struct rte_kni *kni, struct rte_eth_link *link);
+
/**
* Close KNI device.
*/
@@ -15,3 +15,9 @@ DPDK_2.0 {
local: *;
};
+
+EXPERIMENTAL {
+ global:
+
+ rte_kni_update_link;
+};
@@ -118,6 +118,97 @@ kni_change_mtu(uint16_t port_id, unsigned int new_mtu)
port_id, kni_pkt_mtu);
return 0;
}
+
+static int
+kni_change_link(void)
+{
+ struct rte_eth_link link;
+ int ret;
+
+ link.link_speed = 10;
+ link.link_status = ETH_LINK_UP;
+ link.link_autoneg = ETH_LINK_AUTONEG;
+ link.link_duplex = ETH_LINK_FULL_DUPLEX;
+ ret = rte_kni_update_link(test_kni_ctx, &link);
+ if (ret != 0) {
+ printf("Failed to change link state to "
+ "Up/10Mbps/AutoNeg/Full ret=%d.\n", ret);
+ return -1;
+ }
+ rte_delay_ms(1000);
+
+ link.link_speed = 0;
+ link.link_status = ETH_LINK_DOWN;
+ link.link_autoneg = ETH_LINK_FIXED;
+ link.link_duplex = ETH_LINK_FULL_DUPLEX;
+ ret = rte_kni_update_link(test_kni_ctx, &link);
+ if (ret != 0) {
+ printf("Failed to change link state to Down ret=%d.\n", ret);
+ return -1;
+ }
+ rte_delay_ms(1000);
+
+ link.link_speed = 1000;
+ link.link_status = ETH_LINK_UP;
+ link.link_autoneg = ETH_LINK_AUTONEG;
+ link.link_duplex = ETH_LINK_HALF_DUPLEX;
+ ret = rte_kni_update_link(test_kni_ctx, &link);
+ if (ret != 0) {
+ printf("Failed to change link state to "
+ "Up/1Gbps/AutoNeg/Half ret=%d.\n", ret);
+ return -1;
+ }
+ rte_delay_ms(1000);
+
+ link.link_speed = 0;
+ link.link_status = ETH_LINK_DOWN;
+ link.link_autoneg = ETH_LINK_FIXED;
+ link.link_duplex = ETH_LINK_FULL_DUPLEX;
+ ret = rte_kni_update_link(test_kni_ctx, &link);
+ if (ret != 0) {
+ printf("Failed to change link state to Down ret=%d.\n", ret);
+ return -1;
+ }
+ rte_delay_ms(1000);
+
+ link.link_speed = 40000;
+ link.link_status = ETH_LINK_UP;
+ link.link_autoneg = ETH_LINK_FIXED;
+ link.link_duplex = ETH_LINK_FULL_DUPLEX;
+ ret = rte_kni_update_link(test_kni_ctx, &link);
+ if (ret != 0) {
+ printf("Failed to change link state to "
+ "Up/40Gbps/Fixed/Full ret=%d.\n", ret);
+ return -1;
+ }
+ rte_delay_ms(1000);
+
+ link.link_speed = 0;
+ link.link_status = ETH_LINK_DOWN;
+ link.link_autoneg = ETH_LINK_FIXED;
+ link.link_duplex = ETH_LINK_FULL_DUPLEX;
+ ret = rte_kni_update_link(test_kni_ctx, &link);
+ if (ret != 0) {
+ printf("Failed to change link state to Down ret=%d.\n", ret);
+ return -1;
+ }
+ rte_delay_ms(1000);
+
+ link.link_speed = 100000;
+ link.link_status = ETH_LINK_UP;
+ link.link_autoneg = ETH_LINK_FIXED;
+ link.link_duplex = ETH_LINK_FULL_DUPLEX;
+ ret = rte_kni_update_link(test_kni_ctx, &link);
+ if (ret != 0) {
+ printf("Failed to change link state to "
+ "Up/100Gbps/Fixed/Full ret=%d.\n", ret);
+ return -1;
+ }
+ rte_delay_ms(1000);
+
+ return 0;
+}
+
/**
* This loop fully tests the basic functions of KNI. e.g. transmitting,
* receiving to, from kernel space, and kernel requests.
@@ -148,9 +239,16 @@ test_kni_loop(__rte_unused void *arg)
ret = -1;
if (system(IFCONFIG TEST_KNI_PORT" mtu 1400") == -1)
ret = -1;
+
+ ret = kni_change_link();
+ if (ret != 0) {
+ test_kni_processing_flag = 1;
+ return ret;
+ }
+ rte_delay_ms(KNI_TIMEOUT_MS);
if (system(IFCONFIG TEST_KNI_PORT" down") == -1)
ret = -1;
- rte_delay_ms(KNI_TIMEOUT_MS);
+ rte_delay_ms(1000);
test_kni_processing_flag = 1;
} else if (lcore_id == lcore_ingress) {
struct rte_mempool *mp = test_kni_lookup_mempool();