From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id ECF3AA0613 for ; Wed, 25 Sep 2019 11:00:54 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 3860C2BB5; Wed, 25 Sep 2019 11:00:53 +0200 (CEST) Received: from mail-wm1-f65.google.com (mail-wm1-f65.google.com [209.85.128.65]) by dpdk.org (Postfix) with ESMTP id C4C432BAB for ; Wed, 25 Sep 2019 11:00:51 +0200 (CEST) Received: by mail-wm1-f65.google.com with SMTP id b24so3706439wmj.5 for ; Wed, 25 Sep 2019 02:00:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nfware-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=VKcXHyqdeL4t+qTRhCAc0JO9Wa0QoP+iJ1iqDqWxmUA=; b=BcRUkj4XgAuOh8W4BEAGKUikpDimw09OyFZpJJE3avrgwp1jLWTbEptwhCMhc83Zd3 OoeXk0NxZA0G0qr6iaD37HDzVLKm2e2CZQm/fmCqXnl8VCGbB4/dJbGChTBqdvYlDh3W hz9J4SLLhOBczxIC07za5QU24JUEv9YW9je4EFMchLtD5eSBqxTuPjwLwNF/Pts7Ub1t K1UJ4ILBN0Ui5ZU2y+t+H4knX8+dW7auxAujnHXKzjfOrMrO4vtEwpW3OYcmdXO+IiiZ p24zncXrS+OJYoxqo91HA+a0ILj8pbAFAbWDvsnvXCErhG4zuBRfp+clO++c3CGcnT2K 20yQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=VKcXHyqdeL4t+qTRhCAc0JO9Wa0QoP+iJ1iqDqWxmUA=; b=kIqihAVEeIK11ahwPSNaw2S7VvvnUv56UiJI5gJe/vg3n3FR0Oj/CAyQLuHhr8zxGh v72Qk4prJNLp5p1CObvdYMt5uCtELcGGD5o3rxfwP0lAe5qJ0d/zdYPqOwIoxwNBYwAr XmarKcipKPyHcvvFR2uS3dseCrhh/l23jsS/tU+eqIJ2eHZOWzGSmYEFV0Vs9ffIV5nn UIaABKEd+RjHFVQFv19rGZW6Cwee2Km3fyyb2gXqIuTYY2FDLv2Pew0O+xAt5RRkqSbj uCUjH4SAzNcyAXI2OBRWKtgIuePq8ERsHVptOaiSDF6189Cw1Oi6RfQM1IBf3CtTg4jh PRmw== X-Gm-Message-State: APjAAAVV+ic6+pWa3K4d3PqvcOBZjEBMVOINIlH0OIB789mwu/l0J+8c XPccOu0Pq8RIVslr0qfrIpwuedNkkOPZLwRyMZDZR88BaD7zrw== X-Google-Smtp-Source: APXvYqyLxvkKoCucwzt1X+ohqdmoIVcF2pvS/Kh0ggijoBoyREtMIZMG/U2+gnLZHQqOs4jw+cmlm2Lfmn8CTktAF0M= X-Received: by 2002:a7b:c4c7:: with SMTP id g7mr6202768wmk.11.1569402051062; Wed, 25 Sep 2019 02:00:51 -0700 (PDT) MIME-Version: 1.0 References: <20190924192548.17291-1-iryzhov@nfware.com> <20190924193312.17381-1-iryzhov@nfware.com> In-Reply-To: From: Igor Ryzhov Date: Wed, 25 Sep 2019 12:00:39 +0300 Message-ID: To: Aaron Conole Cc: dev@dpdk.org Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH v2] kni: rework rte_kni_update_link using ioctl X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Sure, my bad, sorry. Will send v3. On Tue, Sep 24, 2019 at 11:37 PM Aaron Conole wrote: > Igor Ryzhov writes: > > > Current implementation doesn't allow us to update KNI carrier if the > > interface is not yet UP in kernel. It means that we can't use it in the > > same thread which is processing rte_kni_ops.config_network_if, which is > > very convenient, because it allows us to have correct carrier status > > of the interface right after we enabled it and we don't have to use any > > additional thread to track link status. > > > > Signed-off-by: Igor Ryzhov > > --- > > v2: fix checkpatch warnings > > > > kernel/linux/kni/compat.h | 4 -- > > kernel/linux/kni/kni_misc.c | 42 +++++++++++++++++++ > > kernel/linux/kni/kni_net.c | 15 ------- > > .../linux/eal/include/rte_kni_common.h | 6 +++ > > lib/librte_kni/rte_kni.c | 28 +++---------- > > lib/librte_kni/rte_kni.h | 3 +- > > 6 files changed, 55 insertions(+), 43 deletions(-) > > > > diff --git a/kernel/linux/kni/compat.h b/kernel/linux/kni/compat.h > > index fe0ee55e7..e0a491bcd 100644 > > --- a/kernel/linux/kni/compat.h > > +++ b/kernel/linux/kni/compat.h > > @@ -61,10 +61,6 @@ > > #define kni_sock_map_fd(s) sock_map_fd(s, 0) > > #endif > > > > -#if LINUX_VERSION_CODE >= KERNEL_VERSION(3, 9, 0) > > -#define HAVE_CHANGE_CARRIER_CB > > -#endif > > - > > #if LINUX_VERSION_CODE < KERNEL_VERSION(3, 14, 0) > > #define ether_addr_copy(dst, src) memcpy(dst, src, ETH_ALEN) > > #endif > > diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c > > index aeb275329..9bcba68c8 100644 > > --- a/kernel/linux/kni/kni_misc.c > > +++ b/kernel/linux/kni/kni_misc.c > > @@ -462,6 +462,45 @@ kni_ioctl_release(struct net *net, uint32_t > ioctl_num, > > return ret; > > } > > > > +static int > > +kni_ioctl_link(struct net *net, uint32_t ioctl_num, > > + unsigned long ioctl_param) > > +{ > > + struct kni_net *knet = net_generic(net, kni_net_id); > > + int ret = -EINVAL; > > + struct kni_dev *dev, *n; > > + struct rte_kni_link_info link_info; > > + struct net_device *netdev; > > + > > + if (_IOC_SIZE(ioctl_num) > sizeof(link_info)) > > + return -EINVAL; > > + > > + if (copy_from_user(&link_info, (void *)ioctl_param, > sizeof(link_info))) > > + return -EFAULT; > > + > > + if (strlen(link_info.name) == 0) > > + return -EINVAL; > > + > > + down_read(&knet->kni_list_lock); > > + list_for_each_entry_safe(dev, n, &knet->kni_list_head, list) { > > + if (strncmp(dev->name, link_info.name, RTE_KNI_NAMESIZE) > != 0) > > + continue; > > + > > + netdev = dev->net_dev; > > + > > + if (link_info.linkup) > > + netif_carrier_on(netdev); > > + else > > + netif_carrier_off(netdev); > > + > > + ret = 0; > > + break; > > + } > > + up_read(&knet->kni_list_lock); > > + > > + return ret; > > +} > > + > > static int > > kni_ioctl(struct inode *inode, uint32_t ioctl_num, unsigned long > ioctl_param) > > { > > @@ -483,6 +522,9 @@ kni_ioctl(struct inode *inode, uint32_t ioctl_num, > unsigned long ioctl_param) > > case _IOC_NR(RTE_KNI_IOCTL_RELEASE): > > ret = kni_ioctl_release(net, ioctl_num, ioctl_param); > > break; > > + case _IOC_NR(RTE_KNI_IOCTL_LINK): > > + ret = kni_ioctl_link(net, ioctl_num, ioctl_param); > > + break; > > default: > > pr_debug("IOCTL default\n"); > > break; > > diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c > > index 7bd3a9f1e..cd852eea3 100644 > > --- a/kernel/linux/kni/kni_net.c > > +++ b/kernel/linux/kni/kni_net.c > > @@ -706,18 +706,6 @@ kni_net_set_mac(struct net_device *netdev, void *p) > > return (ret == 0 ? req.result : ret); > > } > > > > -#ifdef HAVE_CHANGE_CARRIER_CB > > -static int > > -kni_net_change_carrier(struct net_device *dev, bool new_carrier) > > -{ > > - if (new_carrier) > > - netif_carrier_on(dev); > > - else > > - netif_carrier_off(dev); > > - return 0; > > -} > > -#endif > > - > > static const struct header_ops kni_net_header_ops = { > > .create = kni_net_header, > > .parse = eth_header_parse, > > @@ -736,9 +724,6 @@ static const struct net_device_ops > kni_net_netdev_ops = { > > .ndo_change_mtu = kni_net_change_mtu, > > .ndo_tx_timeout = kni_net_tx_timeout, > > .ndo_set_mac_address = kni_net_set_mac, > > -#ifdef HAVE_CHANGE_CARRIER_CB > > - .ndo_change_carrier = kni_net_change_carrier, > > -#endif > > }; > > > > static void kni_get_drvinfo(struct net_device *dev, > > diff --git a/lib/librte_eal/linux/eal/include/rte_kni_common.h > b/lib/librte_eal/linux/eal/include/rte_kni_common.h > > index 70992d835..07a10dd93 100644 > > --- a/lib/librte_eal/linux/eal/include/rte_kni_common.h > > +++ b/lib/librte_eal/linux/eal/include/rte_kni_common.h > > @@ -125,10 +125,16 @@ struct rte_kni_device_info { > > uint8_t mac_addr[6]; > > }; > > > > +struct rte_kni_link_info { > > + char name[RTE_KNI_NAMESIZE]; > > + unsigned int linkup; > > +}; > > + > > #define KNI_DEVICE "kni" > > > > #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 _IOWR(0, 4, struct rte_kni_link_info) > > > > #endif /* _RTE_KNI_COMMON_H_ */ > > diff --git a/lib/librte_kni/rte_kni.c b/lib/librte_kni/rte_kni.c > > index 521db27c4..bdea03d5c 100644 > > --- a/lib/librte_kni/rte_kni.c > > +++ b/lib/librte_kni/rte_kni.c > > @@ -738,36 +738,20 @@ rte_kni_update_link(struct rte_kni *kni, unsigned > int linkup) > > You'vbe dropped a lot of code from here, but no the corresponding > variables. This will trigger lots of warnings. Please build with all > warnings and fix them. > > > const char *new_carrier; > > int old_linkup; > > int fd, ret; > > + struct rte_kni_link_info link_info; > > > > if (kni == NULL) > > return -1; > > > > - snprintf(path, sizeof(path), "/sys/devices/virtual/net/%s/carrier", > > - kni->name); > > + snprintf(link_info.name, RTE_KNI_NAMESIZE, "%s", kni->name); > > + link_info.linkup = linkup; > > > > - fd = open(path, O_RDWR); > > - if (fd == -1) { > > - RTE_LOG(ERR, KNI, "Failed to open file: %s.\n", path); > > + if (ioctl(kni_fd, RTE_KNI_IOCTL_LINK, &link_info) < 0) { > > + RTE_LOG(ERR, KNI, "Fail to update KNI link\n"); > > return -1; > > } > > > > - ret = read(fd, old_carrier, 2); > > - if (ret < 1) { > > - close(fd); > > - return -1; > > - } > > - old_linkup = (old_carrier[0] == '1'); > > - > > - new_carrier = linkup ? "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; > > - } > > - > > - close(fd); > > - return old_linkup; > > + return 0; > > } > > > > void > > diff --git a/lib/librte_kni/rte_kni.h b/lib/librte_kni/rte_kni.h > > index b22446fa7..7e1cce1e0 100644 > > --- a/lib/librte_kni/rte_kni.h > > +++ b/lib/librte_kni/rte_kni.h > > @@ -247,9 +247,8 @@ int rte_kni_unregister_handlers(struct rte_kni *kni); > > * > 0 for linkup. > > * > > * @return > > + * On success: 0 > > * On failure: -1 > > - * Previous link state == linkdown: 0 > > - * Previous link state == linkup: 1 > > */ > > __rte_experimental > > int >