From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f68.google.com (mail-wm0-f68.google.com [74.125.82.68]) by dpdk.org (Postfix) with ESMTP id 16C176CA3 for ; Wed, 14 Mar 2018 18:11:25 +0100 (CET) Received: by mail-wm0-f68.google.com with SMTP id u10so5292130wmu.4 for ; Wed, 14 Mar 2018 10:11:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=+D/rKEUECR8wwHK+yCDRE3gJlboswLJVpt1m7jPO/7I=; b=oOMdg+IQcIRp9xlqpkP/Luy2kLSEGN8WSciLIg/nffLXo7jubTpx35KKMwZopoPwpB 3j5gTd9dfTSdSuNFdBrKrzQAHXEMnQbQC3AgiuUh0yq8tC+NDlt1n8NYuJjAH0Ma/Twl 2YjtwU79yBpRgDIierhdykO/YujSlQWuIp8zuL3qlZEiR+SHWS6euucbvjBnu5WpmW+L Q3NCdDr2oMzLnSMd7YS0JZ8oYWLk/3CFuEqLT3phQSLQw16Vt3gbkzf0DsKRpvnb0V23 ONMXtaGnY+RxYBo4xghsfte7guyj+oySU76EaovombyIqORTD9Mqagz6YgpbM1B2xG8a qisQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=+D/rKEUECR8wwHK+yCDRE3gJlboswLJVpt1m7jPO/7I=; b=bYxhR0fHuzoLpqFp6+i/B2mJemWz+ZD9hZ6skHrzwzvFz/2+lrE/RTOxrO/YcK1YRU 7vOQfMQ0flV8Ykp6lUPkkjZ4ocWFxJ0pieNRi/b7VjZmXawuF7OpC50PHXaja9lLUn3g at47MDEOrzJk8tcr5Nyy19LS5maSQdEyV826OD9YmSgK/kn7ZMrW3w2g1jtVs4t2QhNP n3dHOfjssYNkkt5vI2IXSC+0d9NjSrf9vuW5L/R5TyI0ZTcsBXAcoWgUw1vlW6h04f4o JI8DA+SH1VZ6/fKt3gNTMMKWgmS8Xjz/JECMtWugoKPr6DBWh6y+5HbGzWG6JBiVWc/y g8Fw== X-Gm-Message-State: AElRT7EutkZ7Xz/ulvNmybYkYQQnonOnQtxKuMcKJl3Es1xBTBdgqnVV ZMIYXsk0ZbWIC4Di3Ny1FX8HPg== X-Google-Smtp-Source: AG47ELt3upa5sOhWNvI3PxqxpSJwNmzyqIUgeaxcEMRW2tsA8FaTbHTH5TSVAuykL0VvugfPDvs8wg== X-Received: by 10.28.196.143 with SMTP id u137mr2172911wmf.74.1521047485491; Wed, 14 Mar 2018 10:11:25 -0700 (PDT) Received: from 6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id 2sm2149113wmk.29.2018.03.14.10.11.24 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 14 Mar 2018 10:11:24 -0700 (PDT) Date: Wed, 14 Mar 2018 18:11:11 +0100 From: Adrien Mazarguil To: Nelio Laranjeiro Cc: Yongseok Koh , dev@dpdk.org Message-ID: <20180314171111.GO3994@6wind.com> References: <9236b53dbd8149ae9969b8daa359e9ea1facf2d3.1520944256.git.nelio.laranjeiro@6wind.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <9236b53dbd8149ae9969b8daa359e9ea1facf2d3.1520944256.git.nelio.laranjeiro@6wind.com> Subject: Re: [dpdk-dev] [PATCH 4/5] net/mlx5: use Netlink to enable promisc/allmulti 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: , X-List-Received-Date: Wed, 14 Mar 2018 17:11:27 -0000 On Tue, Mar 13, 2018 at 01:50:38PM +0100, Nelio Laranjeiro wrote: > VF devices are not able to receive promisc or allmulti traffic unless it > fully requests it though Netlink. This will cause the request to be > processed by the PF which will handle the request and enable it. > > This requires the VF to be trusted by the PF. > > Signed-off-by: Nelio Laranjeiro Usual comments regarding "vf" => "nl", caps and typos. More nits below. > --- > drivers/net/mlx5/mlx5.h | 2 + > drivers/net/mlx5/mlx5_trigger.c | 27 ++++++++++- > drivers/net/mlx5/mlx5_vf.c | 102 ++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 130 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h > index a4333e6a5..245235641 100644 > --- a/drivers/net/mlx5/mlx5.h > +++ b/drivers/net/mlx5/mlx5.h > @@ -303,5 +303,7 @@ int mlx5_mr_verify(struct rte_eth_dev *dev); > int mlx5_vf_mac_addr_add(struct rte_eth_dev *dev, struct ether_addr *mac); > int mlx5_vf_mac_addr_remove(struct rte_eth_dev *dev, struct ether_addr *mac); > int mlx5_vf_mac_addr_flush(struct rte_eth_dev *dev); > +int mlx5_vf_promisc(struct rte_eth_dev *dev, int enable); > +int mlx5_vf_allmulti(struct rte_eth_dev *dev, int enable); > > #endif /* RTE_PMD_MLX5_H_ */ > diff --git a/drivers/net/mlx5/mlx5_trigger.c b/drivers/net/mlx5/mlx5_trigger.c > index 6bb4ffb14..3f21797ff 100644 > --- a/drivers/net/mlx5/mlx5_trigger.c > +++ b/drivers/net/mlx5/mlx5_trigger.c > @@ -278,8 +278,23 @@ mlx5_traffic_enable(struct rte_eth_dev *dev) > unsigned int j; > int ret; > > - if (priv->isolated) > + if (priv->isolated) { > + if (priv->config.vf) { > + if (dev->data->promiscuous) { > + ret = mlx5_vf_promisc(dev, 1); > + rte_errno = ret; > + if (ret) > + goto error; > + } > + if (dev->data->all_multicast) { > + ret = mlx5_vf_allmulti(dev, 1); > + rte_errno = ret; > + if (ret) > + goto error; > + } > + } Looks like this block should be run no matter what. You should put it before the check on priv->isolated and drop the two next chunks. Note there seems to be missing rollback code in case of error. > return 0; > + } > if (dev->data->promiscuous) { > struct rte_flow_item_eth promisc = { > .dst.addr_bytes = "\x00\x00\x00\x00\x00\x00", > @@ -287,6 +302,11 @@ mlx5_traffic_enable(struct rte_eth_dev *dev) > .type = 0, > }; > > + if (priv->config.vf) { > + ret = mlx5_vf_promisc(dev, 1); > + if (ret) > + goto error; > + } > ret = mlx5_ctrl_flow(dev, &promisc, &promisc); > if (ret) > goto error; > @@ -298,6 +318,11 @@ mlx5_traffic_enable(struct rte_eth_dev *dev) > .type = 0, > }; > > + if (priv->config.vf) { > + ret = mlx5_vf_allmulti(dev, 1); > + if (ret) > + goto error; > + } > ret = mlx5_ctrl_flow(dev, &multicast, &multicast); > if (ret) > goto error; > diff --git a/drivers/net/mlx5/mlx5_vf.c b/drivers/net/mlx5/mlx5_vf.c > index 3db8ee0eb..cf71e79d9 100644 > --- a/drivers/net/mlx5/mlx5_vf.c > +++ b/drivers/net/mlx5/mlx5_vf.c > @@ -447,3 +447,105 @@ mlx5_vf_mac_addr_flush(struct rte_eth_dev *dev) > } > return 0; > } > + > +/** > + * Enable promisc/allmulti though Netlink though => through, also missing period. I suggest to write promisc and allmulti in full in documentation (as "promiscuous" and "all multicast" modes). > + * > + * @param dev > + * Pointer to Ethernet device structure. > + * @param flags > + * IFF_PROMISC for promiscuous, IFF_ALLMULTI for allmulti. > + * @param en en => enable > + * 1 to enable, 0 to disable. => Nonzero to enable, disable otherwise. > + * > + * @return > + * 0 on success, a negative errno value otherwise and rte_errno is set. > + */ > +static int > +mlx5_vf_device_flags(struct rte_eth_dev *dev, uint32_t flags, int enable) > +{ > + int iface_idx = mlx5_vf_iface_idx(dev); > + struct { > + struct nlmsghdr hdr; > + struct ifinfomsg ifi; > + } req = { > + .hdr = { > + .nlmsg_len = NLMSG_LENGTH(sizeof(struct ifinfomsg)), > + .nlmsg_type = RTM_NEWLINK, > + .nlmsg_flags = NLM_F_REQUEST, > + }, > + .ifi = { > + .ifi_flags = enable ? flags : 0, > + .ifi_change = flags, > + .ifi_index = iface_idx, > + }, > + }; > + int fd; > + int ret; > + > + assert(!(flags & ~(IFF_PROMISC | IFF_ALLMULTI))); > + fd = rte_nl_init(NETLINK_ROUTE); > + if (fd < 0) { > + rte_errno = errno; > + goto error; > + } > + ret = rte_nl_send(fd, &req.hdr); > + if (ret == -1) { > + rte_errno = errno; > + goto error; > + } > + rte_nl_final(fd); > + return 0; > +error: > + if (fd >= 0) > + rte_nl_final(fd); > + return -rte_errno; > +} > + > +/** > + * Enable promisc though Netlink Missing period, another suggestion: promisc => promiscuous mode. > + * > + * @param dev > + * Pointer to Ethernet device structure. > + * @param en > + * 1 to enable promisc, 0 to disable it. => Nonzero to enable, disable otherwise. > + * > + * @return > + * 0 on success, a negative errno value otherwise and rte_errno is set. > + */ > +int > +mlx5_vf_promisc(struct rte_eth_dev *dev, int enable) > +{ > + int ret = mlx5_vf_device_flags(dev, IFF_PROMISC, enable); > + > + if (ret) > + DRV_LOG(DEBUG, > + "port %u cannot %s promisc mode: Netlink error %s", > + dev->data->port_id, enable ? "enable" : "disable", > + strerror(rte_errno)); > + return ret; > +} > + > +/** > + * Enable allmulti though Netlink though => through > + * > + * @param dev > + * Pointer to Ethernet device structure. > + * @param en > + * 1 to enable promisc, 0 to disable it. > + * > + * @return > + * 0 on success, a negative errno value otherwise and rte_errno is set. > + */ > +int > +mlx5_vf_allmulti(struct rte_eth_dev *dev, int enable) > +{ > + int ret = mlx5_vf_device_flags(dev, IFF_ALLMULTI, enable); > + > + if (ret) > + DRV_LOG(DEBUG, > + "port %u cannot %s allmulti mode: Netlink error %s", > + dev->data->port_id, enable ? "enable" : "disable", > + strerror(rte_errno)); > + return ret; > +} > -- > 2.11.0 > -- Adrien Mazarguil 6WIND