| Message ID | 1431992009-13573-1-git-send-email-mmvijay@gmail.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id 592C37EEF; Tue, 19 May 2015 01:34:36 +0200 (CEST) Received: from mail-pd0-f174.google.com (mail-pd0-f174.google.com [209.85.192.174]) by dpdk.org (Postfix) with ESMTP id 8A5DA5A0F for <dev@dpdk.org>; Tue, 19 May 2015 01:34:34 +0200 (CEST) Received: by pdbqa5 with SMTP id qa5so168780699pdb.0 for <dev@dpdk.org>; Mon, 18 May 2015 16:34:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:cc:subject:date:message-id; bh=9Hu8ArXlBZuBmPf214sskUDnEAFt/HBG+Vvl2YG6JFU=; b=jwauZG8FcFfdKrbRqh6ZpXuFMrw37GTXiqSy4+3w3Z+QJ+T6DCJFmkQAjBDIcGqbeh 3eNe5d80h7K9IoWIFmq1Ho2rhDdU9u1YEon6FpGadJhBenH3F0/XSAZaDr7KdND0NAuG pMzgjLlAXy5vwvI8fczvdNLcFsiWbQ+ZccYyRiitSF3L5mJ7FizQekJZ04UlWUb+8231 tcxepEVkfZnTdc3kjydmpX6jv4JUvmWPCQfhEe3fgMOYI7JiST0uGBszdqXlHg4+y1eG qvjBsEG2cwfF37RdXaLXA/Qv178VbjP2gHscrOhm5xUayOXd6HzfzFdPypq0Qto2bb5/ /lzg== X-Received: by 10.70.100.230 with SMTP id fb6mr48471475pdb.29.1431992073815; Mon, 18 May 2015 16:34:33 -0700 (PDT) Received: from localhost.localdomain.com ([144.49.130.148]) by mx.google.com with ESMTPSA id bn7sm11160870pac.22.2015.05.18.16.34.32 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 18 May 2015 16:34:33 -0700 (PDT) From: Vijayakumar Muthuvel Manickam <mmvijay@gmail.com> To: dev@dpdk.org Date: Mon, 18 May 2015 19:33:29 -0400 Message-Id: <1431992009-13573-1-git-send-email-mmvijay@gmail.com> X-Mailer: git-send-email 1.8.1.4 Subject: [dpdk-dev] [PATCH] kni: Add link status update X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK <dev.dpdk.org> List-Unsubscribe: <http://dpdk.org/ml/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://dpdk.org/ml/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <http://dpdk.org/ml/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> |
Commit Message
Vijayakumar Muthuvel Manickam
May 18, 2015, 11:33 p.m. UTC
Add an ioctl command in rte_kni module to enable
DPDK applications to propagate link state changes to
kni virtual interfaces.
Signed-off-by: Vijayakumar Muthuvel Manickam <mmvijay@gmail.com>
---
.../linuxapp/eal/include/exec-env/rte_kni_common.h | 2 ++
lib/librte_eal/linuxapp/kni/kni_misc.c | 39 ++++++++++++++++++++++
lib/librte_kni/rte_kni.c | 18 ++++++++++
lib/librte_kni/rte_kni.h | 17 ++++++++++
4 files changed, 76 insertions(+)
Comments
Hello > -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Vijayakumar > Muthuvel Manickam > Sent: Tuesday, May 19, 2015 7:33 AM > To: dev@dpdk.org > Subject: [dpdk-dev] [PATCH] kni: Add link status update > > Add an ioctl command in rte_kni module to enable DPDK applications to > propagate link state changes to kni virtual interfaces. > > Signed-off-by: Vijayakumar Muthuvel Manickam <mmvijay@gmail.com> > --- > .../linuxapp/eal/include/exec-env/rte_kni_common.h | 2 ++ > lib/librte_eal/linuxapp/kni/kni_misc.c | 39 > ++++++++++++++++++++++ > lib/librte_kni/rte_kni.c | 18 ++++++++++ > lib/librte_kni/rte_kni.h | 17 ++++++++++ > 4 files changed, 76 insertions(+) > > diff --git > a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h > b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h > index 1e55c2d..b68001d 100644 > --- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h > +++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h > @@ -163,6 +163,7 @@ struct rte_kni_device_info { > > /* mbuf size */ > unsigned mbuf_size; > + uint8_t link_state; How about transfer more states from user space to kernel space, such as link speed, duplex, etc? > }; > > #define KNI_DEVICE "kni" > @@ -170,5 +171,6 @@ struct rte_kni_device_info { > #define RTE_KNI_IOCTL_TEST _IOWR(0, 1, int) > #define RTE_KNI_IOCTL_CREATE _IOWR(0, 2, struct > rte_kni_device_info) #define RTE_KNI_IOCTL_RELEASE _IOWR(0, 3, > struct rte_kni_device_info) > +#define RTE_KNI_IOCTL_LINK_UPDATE _IOWR(0, 4, struct > +rte_kni_device_info) > > #endif /* _RTE_KNI_COMMON_H_ */ > diff --git a/lib/librte_eal/linuxapp/kni/kni_misc.c > b/lib/librte_eal/linuxapp/kni/kni_misc.c > index 1935d32..b1015cd 100644 > --- a/lib/librte_eal/linuxapp/kni/kni_misc.c > +++ b/lib/librte_eal/linuxapp/kni/kni_misc.c > @@ -548,6 +548,42 @@ kni_ioctl_release(unsigned int ioctl_num, > unsigned long ioctl_param) } > > static int > +kni_ioctl_update_link_state(unsigned int ioctl_num, unsigned long > +ioctl_param) { > + int ret = -EINVAL; > + struct kni_dev *dev, *n; > + struct rte_kni_device_info dev_info; > + > + if (_IOC_SIZE(ioctl_num) > sizeof(dev_info)) > + return -EINVAL; > + > + ret = copy_from_user(&dev_info, (void *)ioctl_param, > sizeof(dev_info)); > + if (ret) { > + KNI_ERR("copy_from_user in kni_ioctl_update_link_status"); > + return -EIO; > + } > + > + if (strlen(dev_info.name) == 0) > + return ret; > + > + down_write(&kni_list_lock); > + list_for_each_entry_safe(dev, n, &kni_list_head, list) { > + if (strncmp(dev->name, dev_info.name, RTE_KNI_NAMESIZE) != > 0) > + continue; > + > + if (dev_info.link_state == 0) > + netif_carrier_off(dev->net_dev); > + else > + netif_carrier_on(dev->net_dev); > + ret = 0; > + break; > + } > + up_write(&kni_list_lock); > + > + return ret; > +} > + > +static int > kni_ioctl(struct inode *inode, > unsigned int ioctl_num, > unsigned long ioctl_param) > @@ -569,6 +605,9 @@ kni_ioctl(struct inode *inode, > case _IOC_NR(RTE_KNI_IOCTL_RELEASE): > ret = kni_ioctl_release(ioctl_num, ioctl_param); > break; > + case _IOC_NR(RTE_KNI_IOCTL_LINK_UPDATE): > + ret = kni_ioctl_update_link_state(ioctl_num, ioctl_param); > + break; > default: > KNI_DBG("IOCTL default \n"); > break; > diff --git a/lib/librte_kni/rte_kni.c b/lib/librte_kni/rte_kni.c index > 4e70fa0..b6eda8a 100644 > --- a/lib/librte_kni/rte_kni.c > +++ b/lib/librte_kni/rte_kni.c > @@ -512,6 +512,24 @@ rte_kni_release(struct rte_kni *kni) } > > int > +rte_kni_update_link_state(struct rte_kni *kni, uint8_t if_up) { > + struct rte_kni_device_info dev_info; > + > + if (!kni || !kni->in_use) > + return -1; > + > + snprintf(dev_info.name, sizeof(dev_info.name), "%s", kni->name); > + dev_info.link_state = if_up; > + if (ioctl(kni_fd, RTE_KNI_IOCTL_LINK_UPDATE, &dev_info) < 0) { > + RTE_LOG(ERR, KNI, "Fail to update link state\n"); > + return -1; > + } > + > + return 0; > +} > + This new interface should be called at the end of rte_kni_alloc() to notify the link status after a KNI interface is created. Thanks, Helin > +int > rte_kni_handle_request(struct rte_kni *kni) { > unsigned ret; > diff --git a/lib/librte_kni/rte_kni.h b/lib/librte_kni/rte_kni.h index > 98edd72..a1bafd9 100644 > --- a/lib/librte_kni/rte_kni.h > +++ b/lib/librte_kni/rte_kni.h > @@ -167,6 +167,23 @@ extern struct rte_kni *rte_kni_create(uint8_t > port_id, extern int rte_kni_release(struct rte_kni *kni); > > /** > + * Send link state changes to KNI interface in kernel space > + * > + * rte_kni_update_link_state is thread safe. > + * > + * @param kni > + * The pointer to the context of an existent KNI interface. > + * @param if_up > + * interface link status > + * > + * @return > + * - 0 indicates success. > + * - negative value indicates failure. > + */ > + > +extern int rte_kni_update_link_state(struct rte_kni *kni, uint8_t > +if_up); > + > +/** > * It is used to handle the request mbufs sent from kernel space. > * Then analyzes it and calls the specific actions for the specific requests. > * Finally constructs the response mbuf and puts it back to the resp_q. > -- > 1.8.1.4
I agree that this looks like a good facility to have but this is not
the right way to do it.
There are already several facilities to accomplish the same thing.
1. You can use the operstate functionality in kernel to manipulate carrier
from another application (read Documentation/operstate.txt)
2. It is possible with sysfs to change carrier state. The KNI driver just
has to provide an .ndo_change_carrier callback.
Introducing more ioctl's is not a good thing. KNI needs to get rid of
the quick and dirty ioctl approach and follow current best practices
for Linux network drivers. It should really be using netlink instead of ioctl().
Ioctl's have several compatibility issues that make them unacceptable upstream.
There is a standard API for create/destroy with netlink and the custom
ioctl's could be banished into the attic of dead API's.
PS: Has anybody even attempted to get KNI upstream? I can see lots
of style (and security) issues that would need to be fixed.
Any response for my comments? Did I miss anything? - Helin > -----Original Message----- > From: Zhang, Helin > Sent: Tuesday, May 19, 2015 8:54 AM > To: Vijayakumar Muthuvel Manickam; dev@dpdk.org > Subject: RE: [dpdk-dev] [PATCH] kni: Add link status update > > Hello > > > -----Original Message----- > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Vijayakumar > > Muthuvel Manickam > > Sent: Tuesday, May 19, 2015 7:33 AM > > To: dev@dpdk.org > > Subject: [dpdk-dev] [PATCH] kni: Add link status update > > > > Add an ioctl command in rte_kni module to enable DPDK applications to > > propagate link state changes to kni virtual interfaces. > > > > Signed-off-by: Vijayakumar Muthuvel Manickam <mmvijay@gmail.com> > > --- > > .../linuxapp/eal/include/exec-env/rte_kni_common.h | 2 ++ > > lib/librte_eal/linuxapp/kni/kni_misc.c | 39 > > ++++++++++++++++++++++ > > lib/librte_kni/rte_kni.c | 18 ++++++++++ > > lib/librte_kni/rte_kni.h | 17 ++++++++++ > > 4 files changed, 76 insertions(+) > > > > diff --git > > a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h > > b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h > > index 1e55c2d..b68001d 100644 > > --- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h > > +++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h > > @@ -163,6 +163,7 @@ struct rte_kni_device_info { > > > > /* mbuf size */ > > unsigned mbuf_size; > > + uint8_t link_state; > How about transfer more states from user space to kernel space, such as link > speed, duplex, etc? > > > }; > > > > #define KNI_DEVICE "kni" > > @@ -170,5 +171,6 @@ struct rte_kni_device_info { > > #define RTE_KNI_IOCTL_TEST _IOWR(0, 1, int) > > #define RTE_KNI_IOCTL_CREATE _IOWR(0, 2, struct > > rte_kni_device_info) #define RTE_KNI_IOCTL_RELEASE _IOWR(0, 3, struct > > rte_kni_device_info) > > +#define RTE_KNI_IOCTL_LINK_UPDATE _IOWR(0, 4, struct > > +rte_kni_device_info) > > > > #endif /* _RTE_KNI_COMMON_H_ */ > > diff --git a/lib/librte_eal/linuxapp/kni/kni_misc.c > > b/lib/librte_eal/linuxapp/kni/kni_misc.c > > index 1935d32..b1015cd 100644 > > --- a/lib/librte_eal/linuxapp/kni/kni_misc.c > > +++ b/lib/librte_eal/linuxapp/kni/kni_misc.c > > @@ -548,6 +548,42 @@ kni_ioctl_release(unsigned int ioctl_num, > > unsigned long ioctl_param) } > > > > static int > > +kni_ioctl_update_link_state(unsigned int ioctl_num, unsigned long > > +ioctl_param) { > > + int ret = -EINVAL; > > + struct kni_dev *dev, *n; > > + struct rte_kni_device_info dev_info; > > + > > + if (_IOC_SIZE(ioctl_num) > sizeof(dev_info)) > > + return -EINVAL; > > + > > + ret = copy_from_user(&dev_info, (void *)ioctl_param, > > sizeof(dev_info)); > > + if (ret) { > > + KNI_ERR("copy_from_user in kni_ioctl_update_link_status"); > > + return -EIO; > > + } > > + > > + if (strlen(dev_info.name) == 0) > > + return ret; > > + > > + down_write(&kni_list_lock); > > + list_for_each_entry_safe(dev, n, &kni_list_head, list) { > > + if (strncmp(dev->name, dev_info.name, RTE_KNI_NAMESIZE) != > > 0) > > + continue; > > + > > + if (dev_info.link_state == 0) > > + netif_carrier_off(dev->net_dev); > > + else > > + netif_carrier_on(dev->net_dev); > > + ret = 0; > > + break; > > + } > > + up_write(&kni_list_lock); > > + > > + return ret; > > +} > > + > > +static int > > kni_ioctl(struct inode *inode, > > unsigned int ioctl_num, > > unsigned long ioctl_param) > > @@ -569,6 +605,9 @@ kni_ioctl(struct inode *inode, > > case _IOC_NR(RTE_KNI_IOCTL_RELEASE): > > ret = kni_ioctl_release(ioctl_num, ioctl_param); > > break; > > + case _IOC_NR(RTE_KNI_IOCTL_LINK_UPDATE): > > + ret = kni_ioctl_update_link_state(ioctl_num, ioctl_param); > > + break; > > default: > > KNI_DBG("IOCTL default \n"); > > break; > > diff --git a/lib/librte_kni/rte_kni.c b/lib/librte_kni/rte_kni.c index > > 4e70fa0..b6eda8a 100644 > > --- a/lib/librte_kni/rte_kni.c > > +++ b/lib/librte_kni/rte_kni.c > > @@ -512,6 +512,24 @@ rte_kni_release(struct rte_kni *kni) } > > > > int > > +rte_kni_update_link_state(struct rte_kni *kni, uint8_t if_up) { > > + struct rte_kni_device_info dev_info; > > + > > + if (!kni || !kni->in_use) > > + return -1; > > + > > + snprintf(dev_info.name, sizeof(dev_info.name), "%s", kni->name); > > + dev_info.link_state = if_up; > > + if (ioctl(kni_fd, RTE_KNI_IOCTL_LINK_UPDATE, &dev_info) < 0) { > > + RTE_LOG(ERR, KNI, "Fail to update link state\n"); > > + return -1; > > + } > > + > > + return 0; > > +} > > + > This new interface should be called at the end of rte_kni_alloc() to notify the link > status after a KNI interface is created. > > Thanks, > Helin > > > +int > > rte_kni_handle_request(struct rte_kni *kni) { > > unsigned ret; > > diff --git a/lib/librte_kni/rte_kni.h b/lib/librte_kni/rte_kni.h index > > 98edd72..a1bafd9 100644 > > --- a/lib/librte_kni/rte_kni.h > > +++ b/lib/librte_kni/rte_kni.h > > @@ -167,6 +167,23 @@ extern struct rte_kni *rte_kni_create(uint8_t > > port_id, extern int rte_kni_release(struct rte_kni *kni); > > > > /** > > + * Send link state changes to KNI interface in kernel space > > + * > > + * rte_kni_update_link_state is thread safe. > > + * > > + * @param kni > > + * The pointer to the context of an existent KNI interface. > > + * @param if_up > > + * interface link status > > + * > > + * @return > > + * - 0 indicates success. > > + * - negative value indicates failure. > > + */ > > + > > +extern int rte_kni_update_link_state(struct rte_kni *kni, uint8_t > > +if_up); > > + > > +/** > > * It is used to handle the request mbufs sent from kernel space. > > * Then analyzes it and calls the specific actions for the specific requests. > > * Finally constructs the response mbuf and puts it back to the resp_q. > > -- > > 1.8.1.4
Hi Helin, Since Stepthen pointed out ioctl is not the best way to add this facility, I have a patch that will enable link status update through sysfs by implementing .ndo_change_carrier. I will submit it today. I will submit a separate patch for transferring link speed,duplex etc., this week. Thanks, Vijay On Sun, Jun 14, 2015 at 5:57 PM, Zhang, Helin <helin.zhang@intel.com> wrote: > Any response for my comments? Did I miss anything? > > - Helin > > > -----Original Message----- > > From: Zhang, Helin > > Sent: Tuesday, May 19, 2015 8:54 AM > > To: Vijayakumar Muthuvel Manickam; dev@dpdk.org > > Subject: RE: [dpdk-dev] [PATCH] kni: Add link status update > > > > Hello > > > > > -----Original Message----- > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Vijayakumar > > > Muthuvel Manickam > > > Sent: Tuesday, May 19, 2015 7:33 AM > > > To: dev@dpdk.org > > > Subject: [dpdk-dev] [PATCH] kni: Add link status update > > > > > > Add an ioctl command in rte_kni module to enable DPDK applications to > > > propagate link state changes to kni virtual interfaces. > > > > > > Signed-off-by: Vijayakumar Muthuvel Manickam <mmvijay@gmail.com> > > > --- > > > .../linuxapp/eal/include/exec-env/rte_kni_common.h | 2 ++ > > > lib/librte_eal/linuxapp/kni/kni_misc.c | 39 > > > ++++++++++++++++++++++ > > > lib/librte_kni/rte_kni.c | 18 ++++++++++ > > > lib/librte_kni/rte_kni.h | 17 ++++++++++ > > > 4 files changed, 76 insertions(+) > > > > > > diff --git > > > a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h > > > b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h > > > index 1e55c2d..b68001d 100644 > > > --- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h > > > +++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h > > > @@ -163,6 +163,7 @@ struct rte_kni_device_info { > > > > > > /* mbuf size */ > > > unsigned mbuf_size; > > > + uint8_t link_state; > > How about transfer more states from user space to kernel space, such as > link > > speed, duplex, etc? > > > > > }; > > > > > > #define KNI_DEVICE "kni" > > > @@ -170,5 +171,6 @@ struct rte_kni_device_info { > > > #define RTE_KNI_IOCTL_TEST _IOWR(0, 1, int) > > > #define RTE_KNI_IOCTL_CREATE _IOWR(0, 2, struct > > > rte_kni_device_info) #define RTE_KNI_IOCTL_RELEASE _IOWR(0, 3, struct > > > rte_kni_device_info) > > > +#define RTE_KNI_IOCTL_LINK_UPDATE _IOWR(0, 4, struct > > > +rte_kni_device_info) > > > > > > #endif /* _RTE_KNI_COMMON_H_ */ > > > diff --git a/lib/librte_eal/linuxapp/kni/kni_misc.c > > > b/lib/librte_eal/linuxapp/kni/kni_misc.c > > > index 1935d32..b1015cd 100644 > > > --- a/lib/librte_eal/linuxapp/kni/kni_misc.c > > > +++ b/lib/librte_eal/linuxapp/kni/kni_misc.c > > > @@ -548,6 +548,42 @@ kni_ioctl_release(unsigned int ioctl_num, > > > unsigned long ioctl_param) } > > > > > > static int > > > +kni_ioctl_update_link_state(unsigned int ioctl_num, unsigned long > > > +ioctl_param) { > > > + int ret = -EINVAL; > > > + struct kni_dev *dev, *n; > > > + struct rte_kni_device_info dev_info; > > > + > > > + if (_IOC_SIZE(ioctl_num) > sizeof(dev_info)) > > > + return -EINVAL; > > > + > > > + ret = copy_from_user(&dev_info, (void *)ioctl_param, > > > sizeof(dev_info)); > > > + if (ret) { > > > + KNI_ERR("copy_from_user in kni_ioctl_update_link_status"); > > > + return -EIO; > > > + } > > > + > > > + if (strlen(dev_info.name) == 0) > > > + return ret; > > > + > > > + down_write(&kni_list_lock); > > > + list_for_each_entry_safe(dev, n, &kni_list_head, list) { > > > + if (strncmp(dev->name, dev_info.name, RTE_KNI_NAMESIZE) != > > > 0) > > > + continue; > > > + > > > + if (dev_info.link_state == 0) > > > + netif_carrier_off(dev->net_dev); > > > + else > > > + netif_carrier_on(dev->net_dev); > > > + ret = 0; > > > + break; > > > + } > > > + up_write(&kni_list_lock); > > > + > > > + return ret; > > > +} > > > + > > > +static int > > > kni_ioctl(struct inode *inode, > > > unsigned int ioctl_num, > > > unsigned long ioctl_param) > > > @@ -569,6 +605,9 @@ kni_ioctl(struct inode *inode, > > > case _IOC_NR(RTE_KNI_IOCTL_RELEASE): > > > ret = kni_ioctl_release(ioctl_num, ioctl_param); > > > break; > > > + case _IOC_NR(RTE_KNI_IOCTL_LINK_UPDATE): > > > + ret = kni_ioctl_update_link_state(ioctl_num, ioctl_param); > > > + break; > > > default: > > > KNI_DBG("IOCTL default \n"); > > > break; > > > diff --git a/lib/librte_kni/rte_kni.c b/lib/librte_kni/rte_kni.c index > > > 4e70fa0..b6eda8a 100644 > > > --- a/lib/librte_kni/rte_kni.c > > > +++ b/lib/librte_kni/rte_kni.c > > > @@ -512,6 +512,24 @@ rte_kni_release(struct rte_kni *kni) } > > > > > > int > > > +rte_kni_update_link_state(struct rte_kni *kni, uint8_t if_up) { > > > + struct rte_kni_device_info dev_info; > > > + > > > + if (!kni || !kni->in_use) > > > + return -1; > > > + > > > + snprintf(dev_info.name, sizeof(dev_info.name), "%s", kni->name); > > > + dev_info.link_state = if_up; > > > + if (ioctl(kni_fd, RTE_KNI_IOCTL_LINK_UPDATE, &dev_info) < 0) { > > > + RTE_LOG(ERR, KNI, "Fail to update link state\n"); > > > + return -1; > > > + } > > > + > > > + return 0; > > > +} > > > + > > This new interface should be called at the end of rte_kni_alloc() to > notify the link > > status after a KNI interface is created. > > > > Thanks, > > Helin > > > > > +int > > > rte_kni_handle_request(struct rte_kni *kni) { > > > unsigned ret; > > > diff --git a/lib/librte_kni/rte_kni.h b/lib/librte_kni/rte_kni.h index > > > 98edd72..a1bafd9 100644 > > > --- a/lib/librte_kni/rte_kni.h > > > +++ b/lib/librte_kni/rte_kni.h > > > @@ -167,6 +167,23 @@ extern struct rte_kni *rte_kni_create(uint8_t > > > port_id, extern int rte_kni_release(struct rte_kni *kni); > > > > > > /** > > > + * Send link state changes to KNI interface in kernel space > > > + * > > > + * rte_kni_update_link_state is thread safe. > > > + * > > > + * @param kni > > > + * The pointer to the context of an existent KNI interface. > > > + * @param if_up > > > + * interface link status > > > + * > > > + * @return > > > + * - 0 indicates success. > > > + * - negative value indicates failure. > > > + */ > > > + > > > +extern int rte_kni_update_link_state(struct rte_kni *kni, uint8_t > > > +if_up); > > > + > > > +/** > > > * It is used to handle the request mbufs sent from kernel space. > > > * Then analyzes it and calls the specific actions for the specific > requests. > > > * Finally constructs the response mbuf and puts it back to the > resp_q. > > > -- > > > 1.8.1.4 > >
That’s great. Thanks! - Helin From: Vijayakumar Muthuvel Manickam [mailto:mmvijay@gmail.com] Sent: Monday, June 15, 2015 9:11 AM To: Zhang, Helin Cc: thomas.monjalon@6wind.com; dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH] kni: Add link status update Hi Helin, Since Stepthen pointed out ioctl is not the best way to add this facility, I have a patch that will enable link status update through sysfs by implementing .ndo_change_carrier. I will submit it today. I will submit a separate patch for transferring link speed,duplex etc., this week. Thanks, Vijay On Sun, Jun 14, 2015 at 5:57 PM, Zhang, Helin <helin.zhang@intel.com<mailto:helin.zhang@intel.com>> wrote: Any response for my comments? Did I miss anything? - Helin > -----Original Message----- > From: Zhang, Helin > Sent: Tuesday, May 19, 2015 8:54 AM > To: Vijayakumar Muthuvel Manickam; dev@dpdk.org<mailto:dev@dpdk.org> > Subject: RE: [dpdk-dev] [PATCH] kni: Add link status update > > Hello > > > -----Original Message----- > > From: dev [mailto:dev-bounces@dpdk.org<mailto:dev-bounces@dpdk.org>] On Behalf Of Vijayakumar > > Muthuvel Manickam > > Sent: Tuesday, May 19, 2015 7:33 AM > > To: dev@dpdk.org<mailto:dev@dpdk.org> > > Subject: [dpdk-dev] [PATCH] kni: Add link status update > > > > Add an ioctl command in rte_kni module to enable DPDK applications to > > propagate link state changes to kni virtual interfaces. > > > > Signed-off-by: Vijayakumar Muthuvel Manickam <mmvijay@gmail.com<mailto:mmvijay@gmail.com>> > > --- > > .../linuxapp/eal/include/exec-env/rte_kni_common.h | 2 ++ > > lib/librte_eal/linuxapp/kni/kni_misc.c | 39 > > ++++++++++++++++++++++ > > lib/librte_kni/rte_kni.c | 18 ++++++++++ > > lib/librte_kni/rte_kni.h | 17 ++++++++++ > > 4 files changed, 76 insertions(+) > > > > diff --git > > a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h > > b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h > > index 1e55c2d..b68001d 100644 > > --- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h > > +++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h > > @@ -163,6 +163,7 @@ struct rte_kni_device_info { > > > > /* mbuf size */ > > unsigned mbuf_size; > > + uint8_t link_state; > How about transfer more states from user space to kernel space, such as link > speed, duplex, etc? > > > }; > > > > #define KNI_DEVICE "kni" > > @@ -170,5 +171,6 @@ struct rte_kni_device_info { > > #define RTE_KNI_IOCTL_TEST _IOWR(0, 1, int) > > #define RTE_KNI_IOCTL_CREATE _IOWR(0, 2, struct > > rte_kni_device_info) #define RTE_KNI_IOCTL_RELEASE _IOWR(0, 3, struct > > rte_kni_device_info) > > +#define RTE_KNI_IOCTL_LINK_UPDATE _IOWR(0, 4, struct > > +rte_kni_device_info) > > > > #endif /* _RTE_KNI_COMMON_H_ */ > > diff --git a/lib/librte_eal/linuxapp/kni/kni_misc.c > > b/lib/librte_eal/linuxapp/kni/kni_misc.c > > index 1935d32..b1015cd 100644 > > --- a/lib/librte_eal/linuxapp/kni/kni_misc.c > > +++ b/lib/librte_eal/linuxapp/kni/kni_misc.c > > @@ -548,6 +548,42 @@ kni_ioctl_release(unsigned int ioctl_num, > > unsigned long ioctl_param) } > > > > static int > > +kni_ioctl_update_link_state(unsigned int ioctl_num, unsigned long > > +ioctl_param) { > > + int ret = -EINVAL; > > + struct kni_dev *dev, *n; > > + struct rte_kni_device_info dev_info; > > + > > + if (_IOC_SIZE(ioctl_num) > sizeof(dev_info)) > > + return -EINVAL; > > + > > + ret = copy_from_user(&dev_info, (void *)ioctl_param, > > sizeof(dev_info)); > > + if (ret) { > > + KNI_ERR("copy_from_user in kni_ioctl_update_link_status"); > > + return -EIO; > > + } > > + > > + if (strlen(dev_info.name<http://dev_info.name>) == 0) > > + return ret; > > + > > + down_write(&kni_list_lock); > > + list_for_each_entry_safe(dev, n, &kni_list_head, list) { > > + if (strncmp(dev->name, dev_info.name<http://dev_info.name>, RTE_KNI_NAMESIZE) != > > 0) > > + continue; > > + > > + if (dev_info.link_state == 0) > > + netif_carrier_off(dev->net_dev); > > + else > > + netif_carrier_on(dev->net_dev); > > + ret = 0; > > + break; > > + } > > + up_write(&kni_list_lock); > > + > > + return ret; > > +} > > + > > +static int > > kni_ioctl(struct inode *inode, > > unsigned int ioctl_num, > > unsigned long ioctl_param) > > @@ -569,6 +605,9 @@ kni_ioctl(struct inode *inode, > > case _IOC_NR(RTE_KNI_IOCTL_RELEASE): > > ret = kni_ioctl_release(ioctl_num, ioctl_param); > > break; > > + case _IOC_NR(RTE_KNI_IOCTL_LINK_UPDATE): > > + ret = kni_ioctl_update_link_state(ioctl_num, ioctl_param); > > + break; > > default: > > KNI_DBG("IOCTL default \n"); > > break; > > diff --git a/lib/librte_kni/rte_kni.c b/lib/librte_kni/rte_kni.c index > > 4e70fa0..b6eda8a 100644 > > --- a/lib/librte_kni/rte_kni.c > > +++ b/lib/librte_kni/rte_kni.c > > @@ -512,6 +512,24 @@ rte_kni_release(struct rte_kni *kni) } > > > > int > > +rte_kni_update_link_state(struct rte_kni *kni, uint8_t if_up) { > > + struct rte_kni_device_info dev_info; > > + > > + if (!kni || !kni->in_use) > > + return -1; > > + > > + snprintf(dev_info.name<http://dev_info.name>, sizeof(dev_info.name<http://dev_info.name>), "%s", kni->name); > > + dev_info.link_state = if_up; > > + if (ioctl(kni_fd, RTE_KNI_IOCTL_LINK_UPDATE, &dev_info) < 0) { > > + RTE_LOG(ERR, KNI, "Fail to update link state\n"); > > + return -1; > > + } > > + > > + return 0; > > +} > > + > This new interface should be called at the end of rte_kni_alloc() to notify the link > status after a KNI interface is created. > > Thanks, > Helin > > > +int > > rte_kni_handle_request(struct rte_kni *kni) { > > unsigned ret; > > diff --git a/lib/librte_kni/rte_kni.h b/lib/librte_kni/rte_kni.h index > > 98edd72..a1bafd9 100644 > > --- a/lib/librte_kni/rte_kni.h > > +++ b/lib/librte_kni/rte_kni.h > > @@ -167,6 +167,23 @@ extern struct rte_kni *rte_kni_create(uint8_t > > port_id, extern int rte_kni_release(struct rte_kni *kni); > > > > /** > > + * Send link state changes to KNI interface in kernel space > > + * > > + * rte_kni_update_link_state is thread safe. > > + * > > + * @param kni > > + * The pointer to the context of an existent KNI interface. > > + * @param if_up > > + * interface link status > > + * > > + * @return > > + * - 0 indicates success. > > + * - negative value indicates failure. > > + */ > > + > > +extern int rte_kni_update_link_state(struct rte_kni *kni, uint8_t > > +if_up); > > + > > +/** > > * It is used to handle the request mbufs sent from kernel space. > > * Then analyzes it and calls the specific actions for the specific requests. > > * Finally constructs the response mbuf and puts it back to the resp_q. > > -- > > 1.8.1.4
diff --git a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h index 1e55c2d..b68001d 100644 --- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h +++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h @@ -163,6 +163,7 @@ struct rte_kni_device_info { /* mbuf size */ unsigned mbuf_size; + uint8_t link_state; }; #define KNI_DEVICE "kni" @@ -170,5 +171,6 @@ struct rte_kni_device_info { #define RTE_KNI_IOCTL_TEST _IOWR(0, 1, int) #define RTE_KNI_IOCTL_CREATE _IOWR(0, 2, struct rte_kni_device_info) #define RTE_KNI_IOCTL_RELEASE _IOWR(0, 3, struct rte_kni_device_info) +#define RTE_KNI_IOCTL_LINK_UPDATE _IOWR(0, 4, struct rte_kni_device_info) #endif /* _RTE_KNI_COMMON_H_ */ diff --git a/lib/librte_eal/linuxapp/kni/kni_misc.c b/lib/librte_eal/linuxapp/kni/kni_misc.c index 1935d32..b1015cd 100644 --- a/lib/librte_eal/linuxapp/kni/kni_misc.c +++ b/lib/librte_eal/linuxapp/kni/kni_misc.c @@ -548,6 +548,42 @@ kni_ioctl_release(unsigned int ioctl_num, unsigned long ioctl_param) } static int +kni_ioctl_update_link_state(unsigned int ioctl_num, unsigned long ioctl_param) +{ + int ret = -EINVAL; + struct kni_dev *dev, *n; + struct rte_kni_device_info dev_info; + + if (_IOC_SIZE(ioctl_num) > sizeof(dev_info)) + return -EINVAL; + + ret = copy_from_user(&dev_info, (void *)ioctl_param, sizeof(dev_info)); + if (ret) { + KNI_ERR("copy_from_user in kni_ioctl_update_link_status"); + return -EIO; + } + + if (strlen(dev_info.name) == 0) + return ret; + + down_write(&kni_list_lock); + list_for_each_entry_safe(dev, n, &kni_list_head, list) { + if (strncmp(dev->name, dev_info.name, RTE_KNI_NAMESIZE) != 0) + continue; + + if (dev_info.link_state == 0) + netif_carrier_off(dev->net_dev); + else + netif_carrier_on(dev->net_dev); + ret = 0; + break; + } + up_write(&kni_list_lock); + + return ret; +} + +static int kni_ioctl(struct inode *inode, unsigned int ioctl_num, unsigned long ioctl_param) @@ -569,6 +605,9 @@ kni_ioctl(struct inode *inode, case _IOC_NR(RTE_KNI_IOCTL_RELEASE): ret = kni_ioctl_release(ioctl_num, ioctl_param); break; + case _IOC_NR(RTE_KNI_IOCTL_LINK_UPDATE): + ret = kni_ioctl_update_link_state(ioctl_num, ioctl_param); + break; default: KNI_DBG("IOCTL default \n"); break; diff --git a/lib/librte_kni/rte_kni.c b/lib/librte_kni/rte_kni.c index 4e70fa0..b6eda8a 100644 --- a/lib/librte_kni/rte_kni.c +++ b/lib/librte_kni/rte_kni.c @@ -512,6 +512,24 @@ rte_kni_release(struct rte_kni *kni) } int +rte_kni_update_link_state(struct rte_kni *kni, uint8_t if_up) +{ + struct rte_kni_device_info dev_info; + + if (!kni || !kni->in_use) + return -1; + + snprintf(dev_info.name, sizeof(dev_info.name), "%s", kni->name); + dev_info.link_state = if_up; + if (ioctl(kni_fd, RTE_KNI_IOCTL_LINK_UPDATE, &dev_info) < 0) { + RTE_LOG(ERR, KNI, "Fail to update link state\n"); + return -1; + } + + return 0; +} + +int rte_kni_handle_request(struct rte_kni *kni) { unsigned ret; diff --git a/lib/librte_kni/rte_kni.h b/lib/librte_kni/rte_kni.h index 98edd72..a1bafd9 100644 --- a/lib/librte_kni/rte_kni.h +++ b/lib/librte_kni/rte_kni.h @@ -167,6 +167,23 @@ extern struct rte_kni *rte_kni_create(uint8_t port_id, extern int rte_kni_release(struct rte_kni *kni); /** + * Send link state changes to KNI interface in kernel space + * + * rte_kni_update_link_state is thread safe. + * + * @param kni + * The pointer to the context of an existent KNI interface. + * @param if_up + * interface link status + * + * @return + * - 0 indicates success. + * - negative value indicates failure. + */ + +extern int rte_kni_update_link_state(struct rte_kni *kni, uint8_t if_up); + +/** * It is used to handle the request mbufs sent from kernel space. * Then analyzes it and calls the specific actions for the specific requests. * Finally constructs the response mbuf and puts it back to the resp_q.