[dpdk-dev,4/5] net/mlx5: use Netlink to enable promisc/allmulti
Checks
Commit Message
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 <nelio.laranjeiro@6wind.com>
---
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(-)
Comments
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 <nelio.laranjeiro@6wind.com>
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
>
@@ -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_ */
@@ -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;
+ }
+ }
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;
@@ -447,3 +447,105 @@ mlx5_vf_mac_addr_flush(struct rte_eth_dev *dev)
}
return 0;
}
+
+/**
+ * Enable promisc/allmulti though Netlink
+ *
+ * @param dev
+ * Pointer to Ethernet device structure.
+ * @param flags
+ * IFF_PROMISC for promiscuous, IFF_ALLMULTI for allmulti.
+ * @param en
+ * 1 to enable, 0 to disable.
+ *
+ * @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
+ *
+ * @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_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
+ *
+ * @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;
+}