From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ie0-f182.google.com (mail-ie0-f182.google.com [209.85.223.182]) by dpdk.org (Postfix) with ESMTP id A1A23C2FC for ; Mon, 15 Jun 2015 03:11:11 +0200 (CEST) Received: by iecrd14 with SMTP id rd14so21705051iec.3 for ; Sun, 14 Jun 2015 18:11:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=U+K6nkVBg8lRe1PHysqeqpYN4JjqwUD6K4D2XP8+5yM=; b=cR7bvN4NEXdsiPK+9NPfiC04MN6HTWOzRpJ7NWetIEbTyF/qIYA2Iv8D7h+5o0N4YE QiN8EwOfrj7l0q1fcAFtlkUAtV6aVO4h1o/De8S5Hs7RjBCiGfe4nxKlK9ZfVSD6bGdH 1uoo02B24GXtSVydGpOsGFpKxI9i2hgVXq/931Coh4adPNuDWbHAmqPpro1pvODi1818 TAseS3kbElyRZK0KdVW756cP5+HLvW2tr+jaXE5flBNAP/AZbtwJWGe0xv5IOgBw++37 wL1nVF/iBAOfz88zmQ48/y/KqIWLrqFHGk1ZqkO36qWSLreQGVOI9QRJvzFVFwSlHM9E 9IFg== MIME-Version: 1.0 X-Received: by 10.43.169.137 with SMTP id nm9mr26759426icc.82.1434330671124; Sun, 14 Jun 2015 18:11:11 -0700 (PDT) Received: by 10.64.5.234 with HTTP; Sun, 14 Jun 2015 18:11:11 -0700 (PDT) In-Reply-To: References: <1431992009-13573-1-git-send-email-mmvijay@gmail.com> Date: Sun, 14 Jun 2015 18:11:11 -0700 Message-ID: From: Vijayakumar Muthuvel Manickam To: "Zhang, Helin" Content-Type: text/plain; charset=UTF-8 X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Cc: "dev@dpdk.org" Subject: Re: [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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 15 Jun 2015 01:11:12 -0000 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 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 > > > --- > > > .../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 > >