[RFC] kni: support allmulticast mode set

Message ID 20190724153541.101034-1-xiaolong.ye@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series [RFC] kni: support allmulticast mode set |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Xiaolong Ye July 24, 2019, 3:35 p.m. UTC
  This patch adds support to allow users enable/disable allmulticast mode for
kni interface.

This requirement comes from bugzilla 312, more details can refer to:
https://bugs.dpdk.org/show_bug.cgi?id=312

Bugzilla ID: 312

Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>
---
 .../prog_guide/kernel_nic_interface.rst       |  9 ++++++
 kernel/linux/kni/kni_net.c                    | 27 +++++++++++-----
 .../linux/eal/include/rte_kni_common.h        |  2 ++
 lib/librte_kni/rte_kni.c                      | 31 ++++++++++++++++++-
 lib/librte_kni/rte_kni.h                      |  3 ++
 5 files changed, 64 insertions(+), 8 deletions(-)
  

Comments

Ferruh Yigit July 29, 2019, 1:38 p.m. UTC | #1
On 7/24/2019 4:35 PM, Xiaolong Ye wrote:
> This patch adds support to allow users enable/disable allmulticast mode for
> kni interface.
> 
> This requirement comes from bugzilla 312, more details can refer to:
> https://bugs.dpdk.org/show_bug.cgi?id=312
> 
> Bugzilla ID: 312
> 
> Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>

Hi Xiaolong,

Patch lgtm, I think it is too later for this release but I suggest sending v1
for next release.

If there is an ABI breakage, to not block the patch for next release, a
deprecation notice needs to be sent in this release.
Although the patch updates header file, as far as I can see it doesn't break the
ABI, since added fields either last element of the struct or the part of the
union. But to be sure, can you please double check if I am missing something?
  
Xiaolong Ye July 29, 2019, 3:25 p.m. UTC | #2
Hi, Ferruh

On 07/29, Ferruh Yigit wrote:
>On 7/24/2019 4:35 PM, Xiaolong Ye wrote:
>> This patch adds support to allow users enable/disable allmulticast mode for
>> kni interface.
>> 
>> This requirement comes from bugzilla 312, more details can refer to:
>> https://bugs.dpdk.org/show_bug.cgi?id=312
>> 
>> Bugzilla ID: 312
>> 
>> Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>
>
>Hi Xiaolong,
>
>Patch lgtm, I think it is too later for this release but I suggest sending v1
>for next release.

Got it, this patch is not that urgent, we can wait for next release.

>
>If there is an ABI breakage, to not block the patch for next release, a
>deprecation notice needs to be sent in this release.
>Although the patch updates header file, as far as I can see it doesn't break the
>ABI, since added fields either last element of the struct or the part of the
>union. But to be sure, can you please double check if I am missing something?

I don't think there is an ABI break either, I'll double check it anyway.

Thanks,
Xiaolong
  

Patch

diff --git a/doc/guides/prog_guide/kernel_nic_interface.rst b/doc/guides/prog_guide/kernel_nic_interface.rst
index 38369b30e..2fd58e117 100644
--- a/doc/guides/prog_guide/kernel_nic_interface.rst
+++ b/doc/guides/prog_guide/kernel_nic_interface.rst
@@ -235,6 +235,15 @@  application functions:
     will be called which calls ``rte_eth_promiscuous_<enable|disable>()``
     on the specified ``port_id``.
 
+``config_allmulticast``:
+
+    Called when the user changes the allmulticast state of the KNI interface.
+    For example, when the user runs ``ifconfig <ifaceX> [-]allmulti``. If the
+    user sets this callback function to NULL, but sets the ``port_id`` field to
+    a value other than -1, a default callback handler in the rte_kni library
+    ``kni_config_allmulticast()`` will be called which calls
+    ``rte_eth_allmulticast_<enable|disable>()`` on the specified ``port_id``.
+
 In order to run these callbacks, the application must periodically call
 the ``rte_kni_handle_request()`` function.  Any user callback function
 registered will be called directly from ``rte_kni_handle_request()`` so
diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c
index 7bd3a9f1e..f25b1277b 100644
--- a/kernel/linux/kni/kni_net.c
+++ b/kernel/linux/kni/kni_net.c
@@ -617,18 +617,31 @@  kni_net_change_mtu(struct net_device *dev, int new_mtu)
 }
 
 static void
-kni_net_set_promiscusity(struct net_device *netdev, int flags)
+kni_net_change_rx_flags(struct net_device *netdev, int flags)
 {
 	struct rte_kni_request req;
 	struct kni_dev *kni = netdev_priv(netdev);
 
 	memset(&req, 0, sizeof(req));
-	req.req_id = RTE_KNI_REQ_CHANGE_PROMISC;
 
-	if (netdev->flags & IFF_PROMISC)
-		req.promiscusity = 1;
-	else
-		req.promiscusity = 0;
+	if (flags & IFF_ALLMULTI) {
+		req.req_id = RTE_KNI_REQ_CHANGE_ALLMULTI;
+
+		if (netdev->flags & IFF_ALLMULTI)
+			req.allmulti = 1;
+		else
+			req.allmulti = 0;
+	}
+
+	if (flags & IFF_PROMISC) {
+		req.req_id = RTE_KNI_REQ_CHANGE_PROMISC;
+
+		if (netdev->flags & IFF_PROMISC)
+			req.promiscusity = 1;
+		else
+			req.promiscusity = 0;
+	}
+
 	kni_net_process_request(kni, &req);
 }
 
@@ -731,7 +744,7 @@  static const struct net_device_ops kni_net_netdev_ops = {
 	.ndo_open = kni_net_open,
 	.ndo_stop = kni_net_release,
 	.ndo_set_config = kni_net_config,
-	.ndo_change_rx_flags = kni_net_set_promiscusity,
+	.ndo_change_rx_flags = kni_net_change_rx_flags,
 	.ndo_start_xmit = kni_net_tx,
 	.ndo_change_mtu = kni_net_change_mtu,
 	.ndo_tx_timeout = kni_net_tx_timeout,
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 37d9ee8f0..b51fe27bd 100644
--- a/lib/librte_eal/linux/eal/include/rte_kni_common.h
+++ b/lib/librte_eal/linux/eal/include/rte_kni_common.h
@@ -31,6 +31,7 @@  enum rte_kni_req_id {
 	RTE_KNI_REQ_CFG_NETWORK_IF,
 	RTE_KNI_REQ_CHANGE_MAC_ADDR,
 	RTE_KNI_REQ_CHANGE_PROMISC,
+	RTE_KNI_REQ_CHANGE_ALLMULTI,
 	RTE_KNI_REQ_MAX,
 };
 
@@ -45,6 +46,7 @@  struct rte_kni_request {
 		uint8_t if_up;       /**< 1: interface up, 0: interface down */
 		uint8_t mac_addr[6]; /**< MAC address for interface */
 		uint8_t promiscusity;/**< 1: promisc mode enable, 0: disable */
+		uint8_t allmulti;    /**< 1: all-multicast mode enable, 0: disable */
 	};
 	int32_t result;               /**< Result for processing request */
 } __attribute__((__packed__));
diff --git a/lib/librte_kni/rte_kni.c b/lib/librte_kni/rte_kni.c
index 4b51fb4fe..cbeb00e8a 100644
--- a/lib/librte_kni/rte_kni.c
+++ b/lib/librte_kni/rte_kni.c
@@ -488,6 +488,26 @@  kni_config_promiscusity(uint16_t port_id, uint8_t to_on)
 	return 0;
 }
 
+/* default callback for request of configuring allmulticast mode */
+static int
+kni_config_allmulticast(uint16_t port_id, uint8_t to_on)
+{
+	if (!rte_eth_dev_is_valid_port(port_id)) {
+		RTE_LOG(ERR, KNI, "Invalid port id %d\n", port_id);
+		return -EINVAL;
+	}
+
+	RTE_LOG(INFO, KNI, "Configure allmulticast mode of %d to %d\n",
+		port_id, to_on);
+
+	if (to_on)
+		rte_eth_allmulticast_enable(port_id);
+	else
+		rte_eth_allmulticast_disable(port_id);
+
+	return 0;
+}
+
 int
 rte_kni_handle_request(struct rte_kni *kni)
 {
@@ -535,6 +555,14 @@  rte_kni_handle_request(struct rte_kni *kni)
 			req->result = kni_config_promiscusity(
 					kni->ops.port_id, req->promiscusity);
 		break;
+	case RTE_KNI_REQ_CHANGE_ALLMULTI: /* Change ALLMULTICAST MODE */
+		if (kni->ops.config_allmulticast)
+			req->result = kni->ops.config_allmulticast(
+					kni->ops.port_id, req->allmulti);
+		else if (kni->ops.port_id != UINT16_MAX)
+			req->result = kni_config_allmulticast(
+					kni->ops.port_id, req->allmulti);
+		break;
 	default:
 		RTE_LOG(ERR, KNI, "Unknown request id %u\n", req->req_id);
 		req->result = -EINVAL;
@@ -684,7 +712,8 @@  kni_check_request_register(struct rte_kni_ops *ops)
 	if (ops->change_mtu == NULL
 	    && ops->config_network_if == NULL
 	    && ops->config_mac_address == NULL
-	    && ops->config_promiscusity == NULL)
+	    && ops->config_promiscusity == NULL
+	    && ops->config_allmulticast == NULL)
 		return KNI_REQ_NO_REGISTER;
 
 	return KNI_REQ_REGISTERED;
diff --git a/lib/librte_kni/rte_kni.h b/lib/librte_kni/rte_kni.h
index 5699a6443..f6b66c33d 100644
--- a/lib/librte_kni/rte_kni.h
+++ b/lib/librte_kni/rte_kni.h
@@ -48,6 +48,9 @@  struct rte_kni_ops {
 
 	/* Pointer to function of configuring promiscuous mode */
 	int (*config_promiscusity)(uint16_t port_id, uint8_t to_on);
+
+	/* Pointer to function of configuring allmulticast mode */
+	int (*config_allmulticast)(uint16_t port_id, uint8_t to_on);
 };
 
 /**