Re: [dpdk-dev] [PATCH 3/5] net/mlx5: use Netlink to add/remove MAC addresses
On Tue, Mar 13, 2018 at 01:50:37PM +0100, Nelio Laranjeiro wrote:
> VF devices are not able to receive traffic unless it fully requests it
> though Netlink. This will cause the request to be processed by the PF
> which will add/remove the MAC address to the VF table if the VF is trusted.
>
> Signed-off-by: Nelio Laranjeiro
Same basic comments as on the previous patch:
- Check random capitalization in documentation and comments.
- "vf" => "nl" for functions, objects and files that aren't really
VF-specific but involve Netlink.
More below.
> ---
> drivers/net/mlx5/Makefile | 3 +-
> drivers/net/mlx5/mlx5.c | 7 +
> drivers/net/mlx5/mlx5.h | 6 +
> drivers/net/mlx5/mlx5_mac.c | 25 +++-
> drivers/net/mlx5/mlx5_vf.c | 315
>
> 5 files changed, 353 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/mlx5/Makefile b/drivers/net/mlx5/Makefile
> index afda4118f..bbda0d1ff 100644
> --- a/drivers/net/mlx5/Makefile
> +++ b/drivers/net/mlx5/Makefile
> @@ -59,6 +59,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_MLX5_PMD) += mlx5_rss.c
> SRCS-$(CONFIG_RTE_LIBRTE_MLX5_PMD) += mlx5_mr.c
> SRCS-$(CONFIG_RTE_LIBRTE_MLX5_PMD) += mlx5_flow.c
> SRCS-$(CONFIG_RTE_LIBRTE_MLX5_PMD) += mlx5_socket.c
> +SRCS-$(CONFIG_RTE_LIBRTE_MLX5_PMD) += mlx5_vf.c
>
> ifeq ($(CONFIG_RTE_LIBRTE_MLX5_DLOPEN_DEPS),y)
> INSTALL-$(CONFIG_RTE_LIBRTE_MLX5_PMD)-lib += $(LIB_GLUE)
> @@ -84,7 +85,7 @@ LDLIBS += -libverbs -lmlx5
> endif
> LDLIBS += -lrte_eal -lrte_mbuf -lrte_mempool -lrte_ring
> LDLIBS += -lrte_ethdev -lrte_net -lrte_kvargs
> -LDLIBS += -lrte_bus_pci
> +LDLIBS += -lrte_bus_pci -lrte_netlink
>
> # A few warnings cannot be avoided in external headers.
> CFLAGS += -Wno-error=cast-qual
> diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
> index d5cc85d19..e966884bd 100644
> --- a/drivers/net/mlx5/mlx5.c
> +++ b/drivers/net/mlx5/mlx5.c
> @@ -205,6 +205,13 @@ mlx5_dev_close(struct rte_eth_dev *dev)
> rte_free(priv->reta_idx);
> if (priv->primary_socket)
> mlx5_socket_uninit(dev);
> + if (priv->config.vf) {
> + ret = mlx5_vf_mac_addr_flush(dev);
> + if (ret)
> + DRV_LOG(WARNING,
> + "port %u some MAC address remains configured",
> + dev->data->port_id);
> + }
> ret = mlx5_hrxq_ibv_verify(dev);
> if (ret)
> DRV_LOG(WARNING, "port %u some hash Rx queue still remain",
> diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
> index 9191b2949..a4333e6a5 100644
> --- a/drivers/net/mlx5/mlx5.h
> +++ b/drivers/net/mlx5/mlx5.h
> @@ -298,4 +298,10 @@ struct mlx5_mr *mlx5_mr_get(struct rte_eth_dev *dev,
> struct rte_mempool *mp);
> int mlx5_mr_release(struct mlx5_mr *mr);
> int mlx5_mr_verify(struct rte_eth_dev *dev);
>
> +/* mlx5_vf.c */
> +
> +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);
> +
> #endif /* RTE_PMD_MLX5_H_ */
> diff --git a/drivers/net/mlx5/mlx5_mac.c b/drivers/net/mlx5/mlx5_mac.c
> index 01c7ba17a..5137ad11d 100644
> --- a/drivers/net/mlx5/mlx5_mac.c
> +++ b/drivers/net/mlx5/mlx5_mac.c
> @@ -67,11 +67,24 @@ mlx5_get_mac(struct rte_eth_dev *dev, uint8_t
> (*mac)[ETHER_ADDR_LEN])
> void
> mlx5_mac_addr_remove(struct rte_eth_dev *dev, uint32_t index)
> {
> + struct priv *priv = dev->data->dev_private;
> + const int vf = priv->config.vf;
> + int ret;
> +
> assert(index < MLX5_MAX_MAC_ADDRESSES);
> + if (index && vf) {
I don't think checking index is necessary here. mlx5_vf_mac_addr_remove()
will fail if the MAC in question happens to be the last one configured on
the netdevice, but depending on external configuration, it's not necessarily
the same as mac_addrs[0].
Checking "vf" only seems more reliable.
> + ret = mlx5_vf_mac_addr_remove(dev,
> + &dev->data->mac_addrs[index]);
> + if (ret) {
> + DRV_LOG(WARNING,
> + "port %u cannot remove mac address at index %d",
> + dev->data->port_id, index);
> + return;
> + }
> + }
> memset(&dev->data->mac_addrs[index], 0, sizeof(struct ether_addr));
> if (!dev->data->promiscuous) {
> - int ret = mlx5_traffic_restart(dev);
> -
> + ret = mlx5_traffic_restart(dev);
> if (ret)
> DRV_LOG(ERR, "port %u cannot remove mac address: %s",
> dev->data->port_id, strerror(rte_errno));
> @@ -97,6 +110,8 @@ int
> mlx5_mac_addr_add(struct rte_eth_dev *dev, struct ether_addr *mac,
> uint32_t index, uint32_t vmdq __rte_unused)
> {
> + struct priv *priv = dev->data->de
[dpdk-dev] [PATCH 3/5] net/mlx5: use Netlink to add/remove MAC addresses
VF devices are not able to receive traffic unless it fully requests it
though Netlink. This will cause the request to be processed by the PF
which will add/remove the MAC address to the VF table if the VF is trusted.
Signed-off-by: Nelio Laranjeiro
---
drivers/net/mlx5/Makefile | 3 +-
drivers/net/mlx5/mlx5.c | 7 +
drivers/net/mlx5/mlx5.h | 6 +
drivers/net/mlx5/mlx5_mac.c | 25 +++-
drivers/net/mlx5/mlx5_vf.c | 315
5 files changed, 353 insertions(+), 3 deletions(-)
diff --git a/drivers/net/mlx5/Makefile b/drivers/net/mlx5/Makefile
index afda4118f..bbda0d1ff 100644
--- a/drivers/net/mlx5/Makefile
+++ b/drivers/net/mlx5/Makefile
@@ -59,6 +59,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_MLX5_PMD) += mlx5_rss.c
SRCS-$(CONFIG_RTE_LIBRTE_MLX5_PMD) += mlx5_mr.c
SRCS-$(CONFIG_RTE_LIBRTE_MLX5_PMD) += mlx5_flow.c
SRCS-$(CONFIG_RTE_LIBRTE_MLX5_PMD) += mlx5_socket.c
+SRCS-$(CONFIG_RTE_LIBRTE_MLX5_PMD) += mlx5_vf.c
ifeq ($(CONFIG_RTE_LIBRTE_MLX5_DLOPEN_DEPS),y)
INSTALL-$(CONFIG_RTE_LIBRTE_MLX5_PMD)-lib += $(LIB_GLUE)
@@ -84,7 +85,7 @@ LDLIBS += -libverbs -lmlx5
endif
LDLIBS += -lrte_eal -lrte_mbuf -lrte_mempool -lrte_ring
LDLIBS += -lrte_ethdev -lrte_net -lrte_kvargs
-LDLIBS += -lrte_bus_pci
+LDLIBS += -lrte_bus_pci -lrte_netlink
# A few warnings cannot be avoided in external headers.
CFLAGS += -Wno-error=cast-qual
diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index d5cc85d19..e966884bd 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -205,6 +205,13 @@ mlx5_dev_close(struct rte_eth_dev *dev)
rte_free(priv->reta_idx);
if (priv->primary_socket)
mlx5_socket_uninit(dev);
+ if (priv->config.vf) {
+ ret = mlx5_vf_mac_addr_flush(dev);
+ if (ret)
+ DRV_LOG(WARNING,
+ "port %u some MAC address remains configured",
+ dev->data->port_id);
+ }
ret = mlx5_hrxq_ibv_verify(dev);
if (ret)
DRV_LOG(WARNING, "port %u some hash Rx queue still remain",
diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index 9191b2949..a4333e6a5 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -298,4 +298,10 @@ struct mlx5_mr *mlx5_mr_get(struct rte_eth_dev *dev,
struct rte_mempool *mp);
int mlx5_mr_release(struct mlx5_mr *mr);
int mlx5_mr_verify(struct rte_eth_dev *dev);
+/* mlx5_vf.c */
+
+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);
+
#endif /* RTE_PMD_MLX5_H_ */
diff --git a/drivers/net/mlx5/mlx5_mac.c b/drivers/net/mlx5/mlx5_mac.c
index 01c7ba17a..5137ad11d 100644
--- a/drivers/net/mlx5/mlx5_mac.c
+++ b/drivers/net/mlx5/mlx5_mac.c
@@ -67,11 +67,24 @@ mlx5_get_mac(struct rte_eth_dev *dev, uint8_t
(*mac)[ETHER_ADDR_LEN])
void
mlx5_mac_addr_remove(struct rte_eth_dev *dev, uint32_t index)
{
+ struct priv *priv = dev->data->dev_private;
+ const int vf = priv->config.vf;
+ int ret;
+
assert(index < MLX5_MAX_MAC_ADDRESSES);
+ if (index && vf) {
+ ret = mlx5_vf_mac_addr_remove(dev,
+ &dev->data->mac_addrs[index]);
+ if (ret) {
+ DRV_LOG(WARNING,
+ "port %u cannot remove mac address at index %d",
+ dev->data->port_id, index);
+ return;
+ }
+ }
memset(&dev->data->mac_addrs[index], 0, sizeof(struct ether_addr));
if (!dev->data->promiscuous) {
- int ret = mlx5_traffic_restart(dev);
-
+ ret = mlx5_traffic_restart(dev);
if (ret)
DRV_LOG(ERR, "port %u cannot remove mac address: %s",
dev->data->port_id, strerror(rte_errno));
@@ -97,6 +110,8 @@ int
mlx5_mac_addr_add(struct rte_eth_dev *dev, struct ether_addr *mac,
uint32_t index, uint32_t vmdq __rte_unused)
{
+ struct priv *priv = dev->data->dev_private;
+ const int vf = priv->config.vf;
unsigned int i;
assert(index < MLX5_MAX_MAC_ADDRESSES);
@@ -111,6 +126,12 @@ mlx5_mac_addr_add(struct rte_eth_dev *dev, struct
ether_addr *mac,
rte_errno = EADDRINUSE;
return -rte_errno;
}
+ if (index && vf) {
+ int ret = mlx5_vf_mac_addr_add(dev, mac);
+
+ if (ret)
+ return ret;
+ }
dev->data->mac_addrs[index] = *mac;
if (!dev->data->promiscuous)
return mlx5_traffic_restart(dev);
diff --git a/drivers/net/mlx5/mlx5_vf.c b/drivers/net/mlx5/mlx5_vf.c
index 357407f56..3db8ee0eb 100644
--- a/drivers/net/mlx5/mlx5_vf.c
+++
