[dpdk-dev] [RFC PATCH 1/5] kni: change and configure mac address

[dpdk-dev] [RFC PATCH 1/5] kni: change and configure mac address

[Hemant Agrawal]

This patch adds following:
1. option to configure the mac address during create
2. inform usespace, if mac address is being changed in linux

Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
---
 .../linuxapp/eal/include/exec-env/rte_kni_common.h        |  3 +++
 lib/librte_eal/linuxapp/kni/kni_misc.c                    |  6 +++++-
 lib/librte_eal/linuxapp/kni/kni_net.c                     | 15 +++++++++++++--
 lib/librte_kni/rte_kni.c                                  | 12 ++++++++++++
 lib/librte_kni/rte_kni.h                                  |  8 ++++++++
 5 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
index 2ac879f..e9fdc73 100644
--- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
+++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
@@ -80,6 +80,7 @@ enum rte_kni_req_id {
 	RTE_KNI_REQ_UNKNOWN = 0,
 	RTE_KNI_REQ_CHANGE_MTU,
 	RTE_KNI_REQ_CFG_NETWORK_IF,
+	RTE_KNI_REQ_CHANGE_MAC_ADDR,
 	RTE_KNI_REQ_MAX,
 };
 
@@ -92,6 +93,7 @@ struct rte_kni_request {
 	union {
 		uint32_t new_mtu;    /**< New MTU */
 		uint8_t if_up;       /**< 1: interface up, 0: interface down */
+		uint8_t mac_addr[6]; /**< MAC address for interface */
 	};
 	int32_t result;               /**< Result for processing request */
 } __attribute__((__packed__));
@@ -168,6 +170,7 @@ struct rte_kni_device_info {
 
 	/* mbuf size */
 	unsigned mbuf_size;
+	char macaddr[6];              /**< Mac Address */
 };
 
 #define KNI_DEVICE "kni"
diff --git a/lib/librte_eal/linuxapp/kni/kni_misc.c b/lib/librte_eal/linuxapp/kni/kni_misc.c
index 7590f1f..90879fa 100644
--- a/lib/librte_eal/linuxapp/kni/kni_misc.c
+++ b/lib/librte_eal/linuxapp/kni/kni_misc.c
@@ -458,12 +458,16 @@ struct kni_net {
 
 	if (kni->lad_dev)
 		ether_addr_copy(net_dev->dev_addr, kni->lad_dev->dev_addr);
-	else
+	else {
 		/*
 		 * Generate random mac address. eth_random_addr() is the newer
 		 * version of generating mac address in linux kernel.
 		 */
 		random_ether_addr(net_dev->dev_addr);
+		
+		/* todo - check if user supplied mac address is available*/
+		memcpy(net_dev->dev_addr, dev_info.macaddr, ETH_ALEN);
+	}
 
 	ret = register_netdev(net_dev);
 	if (ret) {
diff --git a/lib/librte_eal/linuxapp/kni/kni_net.c b/lib/librte_eal/linuxapp/kni/kni_net.c
index db9f489..866cbdd 100644
--- a/lib/librte_eal/linuxapp/kni/kni_net.c
+++ b/lib/librte_eal/linuxapp/kni/kni_net.c
@@ -668,12 +668,23 @@
 static int
 kni_net_set_mac(struct net_device *netdev, void *p)
 {
+	int ret;
+	struct rte_kni_request req;
 	struct sockaddr *addr = p;
+	struct kni_dev *kni;
+
+	kni = netdev_priv(netdev);
+	memset(&req, 0, sizeof(req));
+	req.req_id = RTE_KNI_REQ_CHANGE_MAC_ADDR;
 
-	if (!is_valid_ether_addr((unsigned char *)(addr->sa_data)))
+	if (!is_valid_ether_addr((unsigned char *)(addr->sa_data))) {
 		return -EADDRNOTAVAIL;
+	}
+	memcpy(req.mac_addr, addr->sa_data, netdev->addr_len);
 	memcpy(netdev->dev_addr, addr->sa_data, netdev->addr_len);
-	return 0;
+	ret = kni_net_process_request(kni, &req);
+
+	return (ret == 0 ? req.result : ret);
 }
 
 #ifdef HAVE_CHANGE_CARRIER_CB
diff --git a/lib/librte_kni/rte_kni.c b/lib/librte_kni/rte_kni.c
index 52fcd4b..d5a717b 100644
--- a/lib/librte_kni/rte_kni.c
+++ b/lib/librte_kni/rte_kni.c
@@ -52,6 +52,10 @@
 
 #define MAX_MBUF_BURST_NUM            32
 
+#ifndef ETH_ADDR_LEN
+#define ETH_ADDR_LEN                  6
+#endif
+
 /* Maximum number of ring entries */
 #define KNI_FIFO_COUNT_MAX     1024
 #define KNI_FIFO_SIZE          (KNI_FIFO_COUNT_MAX * sizeof(void *) + \
@@ -368,6 +372,8 @@ struct rte_kni *
 	dev_info.group_id = conf->group_id;
 	dev_info.mbuf_size = conf->mbuf_size;
 
+	memcpy(dev_info.macaddr, conf->macaddr, ETH_ADDR_LEN);
+
 	snprintf(ctx->name, RTE_KNI_NAMESIZE, "%s", intf_name);
 	snprintf(dev_info.name, RTE_KNI_NAMESIZE, "%s", intf_name);
 
@@ -541,6 +547,11 @@ struct rte_kni *
 			req->result = kni->ops.config_network_if(\
 					kni->ops.port_id, req->if_up);
 		break;
+	case RTE_KNI_REQ_CHANGE_MAC_ADDR: /* Change MAC Address */
+		if (kni->ops.config_mac_address)
+			req->result = kni->ops.config_mac_address(kni->ops.port_id,
+							req->mac_addr);
+		break;
 	default:
 		RTE_LOG(ERR, KNI, "Unknown request id %u\n", req->req_id);
 		req->result = -EINVAL;
@@ -735,6 +746,7 @@ struct rte_kni *
 
 	kni->ops.change_mtu = NULL;
 	kni->ops.config_network_if = NULL;
+	kni->ops.config_mac_address = NULL;
 	return 0;
 }
 void
diff --git a/lib/librte_kni/rte_kni.h b/lib/librte_kni/rte_kni.h
index 37deb47..5d2a233 100644
--- a/lib/librte_kni/rte_kni.h
+++ b/lib/librte_kni/rte_kni.h
@@ -59,6 +59,10 @@
 struct rte_kni;
 struct rte_mbuf;
 
+#ifndef ETH_ADDR_LEN
+#define ETH_ADDR_LEN                  6
+#endif
+
 /**
  * Structure which has the function pointers for KNI interface.
  */
@@ -70,6 +74,9 @@ struct rte_kni_ops {
 
 	/* Pointer to function of configuring network interface */
 	int (*config_network_if)(uint8_t port_id, uint8_t if_up);
+
+	/* Pointer to function of configuring mac address */
+	int (*config_mac_address)(uint8_t port_id, uint8_t mac_addr[6]);
 };
 
 /**
@@ -87,6 +94,7 @@ struct rte_kni_conf {
 	unsigned mbuf_size; /* mbuf size */
 	struct rte_pci_addr addr;
 	struct rte_pci_id id;
+	char macaddr[ETH_ADDR_LEN]; /* MAC address assigned to KNI */
 
 	__extension__
 	uint8_t force_bind : 1; /* Flag to bind kernel thread */
-- 
1.9.1

[dpdk-dev] [RFC PATCH 2/5] kni: add support for promisc mode set

[Hemant Agrawal]

message to userspace on promisc mode change

Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
---
 .../linuxapp/eal/include/exec-env/rte_kni_common.h   |  2 ++
 lib/librte_eal/linuxapp/kni/kni_net.c                | 20 ++++++++++++++++++++
 lib/librte_kni/rte_kni.c                             |  5 +++++
 lib/librte_kni/rte_kni.h                             |  3 +++
 4 files changed, 30 insertions(+)

diff --git a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
index e9fdc73..c04fefd 100644
--- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
+++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
@@ -80,6 +80,7 @@ enum rte_kni_req_id {
 	RTE_KNI_REQ_UNKNOWN = 0,
 	RTE_KNI_REQ_CHANGE_MTU,
 	RTE_KNI_REQ_CFG_NETWORK_IF,
+	RTE_KNI_REQ_CHANGE_PROMISC,
 	RTE_KNI_REQ_CHANGE_MAC_ADDR,
 	RTE_KNI_REQ_MAX,
 };
@@ -94,6 +95,7 @@ struct rte_kni_request {
 		uint32_t new_mtu;    /**< New MTU */
 		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 */
 	};
 	int32_t result;               /**< Result for processing request */
 } __attribute__((__packed__));
diff --git a/lib/librte_eal/linuxapp/kni/kni_net.c b/lib/librte_eal/linuxapp/kni/kni_net.c
index 866cbdd..e4a3296 100644
--- a/lib/librte_eal/linuxapp/kni/kni_net.c
+++ b/lib/librte_eal/linuxapp/kni/kni_net.c
@@ -44,6 +44,9 @@
 
 #define WD_TIMEOUT 5 /*jiffies */
 
+#define PROMISC_ENABLE 1
+#define PROMISC_DISABLE 0
+
 #define KNI_WAIT_RESPONSE_TIMEOUT 300 /* 3 seconds */
 
 /* typedef for rx function */
@@ -603,6 +606,22 @@
 	return (ret == 0) ? req.result : ret;
 }
 
+static void 
+kni_net_set_promiscusity(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 = PROMISC_ENABLE;
+	else
+		req.promiscusity= PROMISC_DISABLE;
+	kni_net_process_request(kni, &req);
+}
+
 /*
  * Checks if the user space application provided the resp message
  */
@@ -711,6 +730,7 @@
 	.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_start_xmit = kni_net_tx,
 	.ndo_change_mtu = kni_net_change_mtu,
 	.ndo_do_ioctl = kni_net_ioctl,
diff --git a/lib/librte_kni/rte_kni.c b/lib/librte_kni/rte_kni.c
index d5a717b..8ae632f 100644
--- a/lib/librte_kni/rte_kni.c
+++ b/lib/librte_kni/rte_kni.c
@@ -552,6 +552,11 @@ struct rte_kni *
 			req->result = kni->ops.config_mac_address(kni->ops.port_id,
 							req->mac_addr);
 		break;
+	case RTE_KNI_REQ_CHANGE_PROMISC: /* Change PROMISCUOUS MODE */
+		if (kni->ops.config_promiscusity)
+			req->result = kni->ops.config_promiscusity(kni->ops.port_id,
+							req->promiscusity);
+		break;
 	default:
 		RTE_LOG(ERR, KNI, "Unknown request id %u\n", req->req_id);
 		req->result = -EINVAL;
diff --git a/lib/librte_kni/rte_kni.h b/lib/librte_kni/rte_kni.h
index 5d2a233..488db4b 100644
--- a/lib/librte_kni/rte_kni.h
+++ b/lib/librte_kni/rte_kni.h
@@ -74,6 +74,9 @@ struct rte_kni_ops {
 
 	/* Pointer to function of configuring network interface */
 	int (*config_network_if)(uint8_t port_id, uint8_t if_up);
+	
+	/* Pointer to function of configuring promiscuous mode */
+	int (*config_promiscusity)(uint8_t port_id, uint8_t to_on);
 
 	/* Pointer to function of configuring mac address */
 	int (*config_mac_address)(uint8_t port_id, uint8_t mac_addr[6]);
-- 
1.9.1

[dpdk-dev] [RFC PATCH 3/5] kni: init and change request for mtu

[Hemant Agrawal]

1. Configure initial mtu.
2. Message to userspace for mtu change

Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
---
 lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h | 1 +
 lib/librte_eal/linuxapp/kni/kni_misc.c                        | 2 ++
 lib/librte_kni/rte_kni.c                                      | 1 +
 lib/librte_kni/rte_kni.h                                      | 1 +
 4 files changed, 5 insertions(+)

diff --git a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
index c04fefd..2cd7d9a 100644
--- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
+++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
@@ -172,6 +172,7 @@ struct rte_kni_device_info {
 
 	/* mbuf size */
 	unsigned mbuf_size;
+	unsigned int mtu;             /**< MTU */
 	char macaddr[6];              /**< Mac Address */
 };
 
diff --git a/lib/librte_eal/linuxapp/kni/kni_misc.c b/lib/librte_eal/linuxapp/kni/kni_misc.c
index 90879fa..4f99a5e 100644
--- a/lib/librte_eal/linuxapp/kni/kni_misc.c
+++ b/lib/librte_eal/linuxapp/kni/kni_misc.c
@@ -469,6 +469,8 @@ struct kni_net {
 		memcpy(net_dev->dev_addr, dev_info.macaddr, ETH_ALEN);
 	}
 
+	net_dev->mtu = dev_info.mtu;
+
 	ret = register_netdev(net_dev);
 	if (ret) {
 		pr_err("error %i registering device \"%s\"\n",
diff --git a/lib/librte_kni/rte_kni.c b/lib/librte_kni/rte_kni.c
index 8ae632f..a3aab9d 100644
--- a/lib/librte_kni/rte_kni.c
+++ b/lib/librte_kni/rte_kni.c
@@ -373,6 +373,7 @@ struct rte_kni *
 	dev_info.mbuf_size = conf->mbuf_size;
 
 	memcpy(dev_info.macaddr, conf->macaddr, ETH_ADDR_LEN);
+	dev_info.mtu = conf->mtu;
 
 	snprintf(ctx->name, RTE_KNI_NAMESIZE, "%s", intf_name);
 	snprintf(dev_info.name, RTE_KNI_NAMESIZE, "%s", intf_name);
diff --git a/lib/librte_kni/rte_kni.h b/lib/librte_kni/rte_kni.h
index 488db4b..3156e33 100644
--- a/lib/librte_kni/rte_kni.h
+++ b/lib/librte_kni/rte_kni.h
@@ -98,6 +98,7 @@ struct rte_kni_conf {
 	struct rte_pci_addr addr;
 	struct rte_pci_id id;
 	char macaddr[ETH_ADDR_LEN]; /* MAC address assigned to KNI */
+	uint16_t mtu;	/* Maximum transmission Unit of KNI*/
 
 	__extension__
 	uint8_t force_bind : 1; /* Flag to bind kernel thread */
-- 
1.9.1

[dpdk-dev] [RFC PATCH 4/5] kni: add support to get gso_size info

[Hemant Agrawal]

Inform userspace about gso size info

Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
---
 lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h | 3 ++-
 lib/librte_eal/linuxapp/kni/kni_net.c                         | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
index 2cd7d9a..91ebed3 100644
--- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
+++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
@@ -125,7 +125,8 @@ struct rte_kni_mbuf {
 	uint16_t nb_segs;       /**< Number of segments. */
 	char pad4[2];
 	uint64_t ol_flags;      /**< Offload features. */
-	char pad2[4];
+	uint16_t gso_size;      /**< TCP Segmentation Offload Information. */
+	char pad2[2];
 	uint32_t pkt_len;       /**< Total pkt len: sum of all segment data_len. */
 	uint16_t data_len;      /**< Amount of data in segment buffer. */
 
diff --git a/lib/librte_eal/linuxapp/kni/kni_net.c b/lib/librte_eal/linuxapp/kni/kni_net.c
index e4a3296..c7648d3 100644
--- a/lib/librte_eal/linuxapp/kni/kni_net.c
+++ b/lib/librte_eal/linuxapp/kni/kni_net.c
@@ -252,6 +252,7 @@
 		}
 		pkt_kva->pkt_len = len;
 		pkt_kva->data_len = len;
+		pkt_kva->gso_size = skb_shinfo(skb)->gso_size; /* passes gso_size from Kernel to GPP */
 
 		/* enqueue mbuf into tx_q */
 		ret = kni_fifo_put(kni->tx_q, &pkt_va, 1);
-- 
1.9.1

[dpdk-dev] [RFC PATCH 5/5] kni: support multiple userspace process working with kni module

[Hemant Agrawal]

in case of multiple application using the same KNI module,
protect that one application will only clean it's own devices.

Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
---
 lib/librte_eal/linuxapp/kni/kni_dev.h  |  1 +
 lib/librte_eal/linuxapp/kni/kni_misc.c | 21 +++++++++++++++------
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/lib/librte_eal/linuxapp/kni/kni_dev.h b/lib/librte_eal/linuxapp/kni/kni_dev.h
index 72385ab..c8c856d 100644
--- a/lib/librte_eal/linuxapp/kni/kni_dev.h
+++ b/lib/librte_eal/linuxapp/kni/kni_dev.h
@@ -61,6 +61,7 @@ struct kni_dev {
 	uint32_t core_id;            /* Core ID to bind */
 	char name[RTE_KNI_NAMESIZE]; /* Network device name */
 	struct task_struct *pthread;
+	void *usrctxt; /* User space process context*/
 
 	/* wait queue for req/resp */
 	wait_queue_head_t wq;
diff --git a/lib/librte_eal/linuxapp/kni/kni_misc.c b/lib/librte_eal/linuxapp/kni/kni_misc.c
index 4f99a5e..84f8a99 100644
--- a/lib/librte_eal/linuxapp/kni/kni_misc.c
+++ b/lib/librte_eal/linuxapp/kni/kni_misc.c
@@ -234,6 +234,9 @@ struct kni_net {
 
 	down_write(&knet->kni_list_lock);
 	list_for_each_entry_safe(dev, n, &knet->kni_list_head, list) {
+		/* check for the matching user context */
+		if (dev->usrctxt != inode)
+			continue;
 		/* Stop kernel thread for multiple mode */
 		if (multiple_kthread_on && dev->pthread != NULL) {
 			kthread_stop(dev->pthread);
@@ -311,9 +314,10 @@ struct kni_net {
 }
 
 static int
-kni_ioctl_create(struct net *net, uint32_t ioctl_num,
+kni_ioctl_create(struct inode *inode, uint32_t ioctl_num,
 		unsigned long ioctl_param)
 {
+	struct net *net = current->nsproxy->net_ns;
 	struct kni_net *knet = net_generic(net, kni_net_id);
 	int ret;
 	struct rte_kni_device_info dev_info;
@@ -374,7 +378,8 @@ struct kni_net {
 	dev_net_set(net_dev, net);
 
 	kni = netdev_priv(net_dev);
-
+	
+	kni->usrctxt = inode;
 	kni->net_dev = net_dev;
 	kni->group_id = dev_info.group_id;
 	kni->core_id = dev_info.core_id;
@@ -493,9 +498,10 @@ struct kni_net {
 }
 
 static int
-kni_ioctl_release(struct net *net, uint32_t ioctl_num,
+kni_ioctl_release(struct inode *inode, uint32_t ioctl_num,
 		unsigned long ioctl_param)
 {
+	struct net *net = current->nsproxy->net_ns;
 	struct kni_net *knet = net_generic(net, kni_net_id);
 	int ret = -EINVAL;
 	struct kni_dev *dev, *n;
@@ -516,6 +522,10 @@ struct kni_net {
 
 	down_write(&knet->kni_list_lock);
 	list_for_each_entry_safe(dev, n, &knet->kni_list_head, list) {
+		/* only the owner user process can remove it*/
+		if (dev->usrctxt != inode)
+			continue;
+
 		if (strncmp(dev->name, dev_info.name, RTE_KNI_NAMESIZE) != 0)
 			continue;
 
@@ -540,7 +550,6 @@ struct kni_net {
 kni_ioctl(struct inode *inode, uint32_t ioctl_num, unsigned long ioctl_param)
 {
 	int ret = -EINVAL;
-	struct net *net = current->nsproxy->net_ns;
 
 	pr_debug("IOCTL num=0x%0x param=0x%0lx\n", ioctl_num, ioctl_param);
 
@@ -552,10 +561,10 @@ struct kni_net {
 		/* For test only, not used */
 		break;
 	case _IOC_NR(RTE_KNI_IOCTL_CREATE):
-		ret = kni_ioctl_create(net, ioctl_num, ioctl_param);
+		ret = kni_ioctl_create(inode, ioctl_num, ioctl_param);
 		break;
 	case _IOC_NR(RTE_KNI_IOCTL_RELEASE):
-		ret = kni_ioctl_release(net, ioctl_num, ioctl_param);
+		ret = kni_ioctl_release(inode, ioctl_num, ioctl_param);
 		break;
 	default:
 		pr_debug("IOCTL default\n");
-- 
1.9.1

Re: [dpdk-dev] [RFC PATCH 4/5] kni: add support to get gso_size info

[Alejandro Lucero]

I guess this is just need this for tso offload. isn't it?

Just asking because I have done some work adding gather and tso offload to
kni and I was wondering if this is duplicated job from my part.

If you are wondering what KNI offload means, since it is not talking to any
hw, the offload is regarding the PMD where KNI is sending packet to. So if
that PMD supports hw offloads, KNI netdev can advertise that to the linux
network stack.



On Wed, May 3, 2017 at 12:21 PM, Hemant Agrawal <hemant.agrawal@nxp.com>
wrote:

> Inform userspace about gso size info
>
> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
> ---
>  lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h | 3 ++-
>  lib/librte_eal/linuxapp/kni/kni_net.c                         | 1 +
>  2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
> b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
> index 2cd7d9a..91ebed3 100644
> --- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
> +++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
> @@ -125,7 +125,8 @@ struct rte_kni_mbuf {
>         uint16_t nb_segs;       /**< Number of segments. */
>         char pad4[2];
>         uint64_t ol_flags;      /**< Offload features. */
> -       char pad2[4];
> +       uint16_t gso_size;      /**< TCP Segmentation Offload Information.
> */
> +       char pad2[2];
>         uint32_t pkt_len;       /**< Total pkt len: sum of all segment
> data_len. */
>         uint16_t data_len;      /**< Amount of data in segment buffer. */
>
> diff --git a/lib/librte_eal/linuxapp/kni/kni_net.c
> b/lib/librte_eal/linuxapp/kni/kni_net.c
> index e4a3296..c7648d3 100644
> --- a/lib/librte_eal/linuxapp/kni/kni_net.c
> +++ b/lib/librte_eal/linuxapp/kni/kni_net.c
> @@ -252,6 +252,7 @@
>                 }
>                 pkt_kva->pkt_len = len;
>                 pkt_kva->data_len = len;
> +               pkt_kva->gso_size = skb_shinfo(skb)->gso_size; /* passes
> gso_size from Kernel to GPP */
>
>                 /* enqueue mbuf into tx_q */
>                 ret = kni_fifo_put(kni->tx_q, &pkt_va, 1);
> --
> 1.9.1
>
>

Re: [dpdk-dev] [RFC PATCH 1/5] kni: change and configure mac address

[Ferruh Yigit]

On 5/3/2017 12:21 PM, Hemant Agrawal wrote:
> This patch adds following:
> 1. option to configure the mac address during create
> 2. inform usespace, if mac address is being changed in linux
> 
> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
> ---
>  .../linuxapp/eal/include/exec-env/rte_kni_common.h        |  3 +++
>  lib/librte_eal/linuxapp/kni/kni_misc.c                    |  6 +++++-
>  lib/librte_eal/linuxapp/kni/kni_net.c                     | 15 +++++++++++++--
>  lib/librte_kni/rte_kni.c                                  | 12 ++++++++++++
>  lib/librte_kni/rte_kni.h                                  |  8 ++++++++
>  5 files changed, 41 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
> index 2ac879f..e9fdc73 100644
> --- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
> +++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
> @@ -80,6 +80,7 @@ enum rte_kni_req_id {
>  	RTE_KNI_REQ_UNKNOWN = 0,
>  	RTE_KNI_REQ_CHANGE_MTU,
>  	RTE_KNI_REQ_CFG_NETWORK_IF,
> +	RTE_KNI_REQ_CHANGE_MAC_ADDR,
>  	RTE_KNI_REQ_MAX,
>  };
>  
> @@ -92,6 +93,7 @@ struct rte_kni_request {
>  	union {
>  		uint32_t new_mtu;    /**< New MTU */
>  		uint8_t if_up;       /**< 1: interface up, 0: interface down */
> +		uint8_t mac_addr[6]; /**< MAC address for interface */
>  	};
>  	int32_t result;               /**< Result for processing request */
>  } __attribute__((__packed__));
> @@ -168,6 +170,7 @@ struct rte_kni_device_info {
>  
>  	/* mbuf size */
>  	unsigned mbuf_size;
> +	char macaddr[6];              /**< Mac Address */

I think it is good to use same variable name for same reason, above uses
"mac_addr" here "macaddr", please pick one.

And perhaps field comment can be removed, it is not adding any extra
information.

>  };
>  
>  #define KNI_DEVICE "kni"
> diff --git a/lib/librte_eal/linuxapp/kni/kni_misc.c b/lib/librte_eal/linuxapp/kni/kni_misc.c
> index 7590f1f..90879fa 100644
> --- a/lib/librte_eal/linuxapp/kni/kni_misc.c
> +++ b/lib/librte_eal/linuxapp/kni/kni_misc.c
> @@ -458,12 +458,16 @@ struct kni_net {
>  
>  	if (kni->lad_dev)
>  		ether_addr_copy(net_dev->dev_addr, kni->lad_dev->dev_addr);
> -	else
> +	else {
>  		/*
>  		 * Generate random mac address. eth_random_addr() is the newer
>  		 * version of generating mac address in linux kernel.
>  		 */
>  		random_ether_addr(net_dev->dev_addr);
> +		
> +		/* todo - check if user supplied mac address is available*/

Can we implement todo J

> +		memcpy(net_dev->dev_addr, dev_info.macaddr, ETH_ALEN);
> +	}
>  
>  	ret = register_netdev(net_dev);
>  	if (ret) {
> diff --git a/lib/librte_eal/linuxapp/kni/kni_net.c b/lib/librte_eal/linuxapp/kni/kni_net.c
> index db9f489..866cbdd 100644
> --- a/lib/librte_eal/linuxapp/kni/kni_net.c
> +++ b/lib/librte_eal/linuxapp/kni/kni_net.c
> @@ -668,12 +668,23 @@
>  static int
>  kni_net_set_mac(struct net_device *netdev, void *p)
>  {
> +	int ret;
> +	struct rte_kni_request req;
>  	struct sockaddr *addr = p;
> +	struct kni_dev *kni;
> +
> +	kni = netdev_priv(netdev);
> +	memset(&req, 0, sizeof(req));
> +	req.req_id = RTE_KNI_REQ_CHANGE_MAC_ADDR;
>  
> -	if (!is_valid_ether_addr((unsigned char *)(addr->sa_data)))
> +	if (!is_valid_ether_addr((unsigned char *)(addr->sa_data))) {

Not required to add {

>  		return -EADDRNOTAVAIL;
> +	}
> +	memcpy(req.mac_addr, addr->sa_data, netdev->addr_len);
>  	memcpy(netdev->dev_addr, addr->sa_data, netdev->addr_len);
> -	return 0;
> +	ret = kni_net_process_request(kni, &req);
> +
> +	return (ret == 0 ? req.result : ret);

If config_mac_address() is not defined, req.result will be undefined.
You should give initial value to req.

>  }
>  
>  #ifdef HAVE_CHANGE_CARRIER_CB
> diff --git a/lib/librte_kni/rte_kni.c b/lib/librte_kni/rte_kni.c
> index 52fcd4b..d5a717b 100644
> --- a/lib/librte_kni/rte_kni.c
> +++ b/lib/librte_kni/rte_kni.c
> @@ -52,6 +52,10 @@
>  
>  #define MAX_MBUF_BURST_NUM            32
>  
> +#ifndef ETH_ADDR_LEN
> +#define ETH_ADDR_LEN                  6
> +#endif

This is redundant, rte_kni.h has something similar.

> +
>  /* Maximum number of ring entries */
>  #define KNI_FIFO_COUNT_MAX     1024
>  #define KNI_FIFO_SIZE          (KNI_FIFO_COUNT_MAX * sizeof(void *) + \
> @@ -368,6 +372,8 @@ struct rte_kni *
>  	dev_info.group_id = conf->group_id;
>  	dev_info.mbuf_size = conf->mbuf_size;
>  
> +	memcpy(dev_info.macaddr, conf->macaddr, ETH_ADDR_LEN);
> +
>  	snprintf(ctx->name, RTE_KNI_NAMESIZE, "%s", intf_name);
>  	snprintf(dev_info.name, RTE_KNI_NAMESIZE, "%s", intf_name);
>  
> @@ -541,6 +547,11 @@ struct rte_kni *
>  			req->result = kni->ops.config_network_if(\
>  					kni->ops.port_id, req->if_up);
>  		break;
> +	case RTE_KNI_REQ_CHANGE_MAC_ADDR: /* Change MAC Address */
> +		if (kni->ops.config_mac_address)
> +			req->result = kni->ops.config_mac_address(kni->ops.port_id,
> +							req->mac_addr);
> +		break;
>  	default:
>  		RTE_LOG(ERR, KNI, "Unknown request id %u\n", req->req_id);
>  		req->result = -EINVAL;
> @@ -735,6 +746,7 @@ struct rte_kni *
>  
>  	kni->ops.change_mtu = NULL;
>  	kni->ops.config_network_if = NULL;
> +	kni->ops.config_mac_address = NULL;

Can do memset here perhaps?

Also need to update kni_check_request_register(), it only checks
change_mtu and config_network_if pointers to decide if ops registered,
since there are more ops now, that logic needs to be updated.

>  	return 0;
>  }
>  void
> diff --git a/lib/librte_kni/rte_kni.h b/lib/librte_kni/rte_kni.h
> index 37deb47..5d2a233 100644
> --- a/lib/librte_kni/rte_kni.h
> +++ b/lib/librte_kni/rte_kni.h
> @@ -59,6 +59,10 @@
>  struct rte_kni;
>  struct rte_mbuf;
>  
> +#ifndef ETH_ADDR_LEN
> +#define ETH_ADDR_LEN                  6
> +#endif
> +

Can include rte_ether.h, and use ETHER_ADDR_LEN here.

>  /**
>   * Structure which has the function pointers for KNI interface.
>   */
> @@ -70,6 +74,9 @@ struct rte_kni_ops {
>  
>  	/* Pointer to function of configuring network interface */
>  	int (*config_network_if)(uint8_t port_id, uint8_t if_up);
> +
> +	/* Pointer to function of configuring mac address */
> +	int (*config_mac_address)(uint8_t port_id, uint8_t mac_addr[6]);

Can replace "6" with ETHER_ADDR_LEN

Two things here:

1) Providing rte_kni_ops interface during creating a KNI interface links
KNI interface a backing DPDK port. And kni_ops applied on to this port.

For the case "config_mac_address", it is obvious that what we want to do
on to DPDK port. Why not provide a default implementation for this
function in librte_kni (via this patch) and set it by default.
Application still can overwrite it if it has something else to do. This
saves effort for others.

2) Can you please update sample application with this new kni_ops, again
in this patch?


>  };
>  
>  /**
> @@ -87,6 +94,7 @@ struct rte_kni_conf {
>  	unsigned mbuf_size; /* mbuf size */
>  	struct rte_pci_addr addr;
>  	struct rte_pci_id id;
> +	char macaddr[ETH_ADDR_LEN]; /* MAC address assigned to KNI */
>  
>  	__extension__
>  	uint8_t force_bind : 1; /* Flag to bind kernel thread */
> 

Re: [dpdk-dev] [RFC PATCH 4/5] kni: add support to get gso_size info

[Ferruh Yigit]

On 5/3/2017 12:21 PM, Hemant Agrawal wrote:
> Inform userspace about gso size info
> 
> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
> ---
>  lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h | 3 ++-
>  lib/librte_eal/linuxapp/kni/kni_net.c                         | 1 +
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
> index 2cd7d9a..91ebed3 100644
> --- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
> +++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
> @@ -125,7 +125,8 @@ struct rte_kni_mbuf {
>  	uint16_t nb_segs;       /**< Number of segments. */
>  	char pad4[2];
>  	uint64_t ol_flags;      /**< Offload features. */
> -	char pad2[4];
> +	uint16_t gso_size;      /**< TCP Segmentation Offload Information. */
> +	char pad2[2];

rte_kni_mbuf and rte_mbuf should be binary compatible. This is
packet_type field of the mbuf struct.

When DPDK application receives the mbuf, how it will know if this
gso_size or packet_type, unless it knows underlying port is KNI. Is it
possible to use udata64 field for this?

Who will use information and how it will be used, can you please provide
some sort of sample?

Thanks,
ferruh

>  	uint32_t pkt_len;       /**< Total pkt len: sum of all segment data_len. */
>  	uint16_t data_len;      /**< Amount of data in segment buffer. */
>  
> diff --git a/lib/librte_eal/linuxapp/kni/kni_net.c b/lib/librte_eal/linuxapp/kni/kni_net.c
> index e4a3296..c7648d3 100644
> --- a/lib/librte_eal/linuxapp/kni/kni_net.c
> +++ b/lib/librte_eal/linuxapp/kni/kni_net.c
> @@ -252,6 +252,7 @@
>  		}
>  		pkt_kva->pkt_len = len;
>  		pkt_kva->data_len = len;
> +		pkt_kva->gso_size = skb_shinfo(skb)->gso_size; /* passes gso_size from Kernel to GPP */
>  
>  		/* enqueue mbuf into tx_q */
>  		ret = kni_fifo_put(kni->tx_q, &pkt_va, 1);
> 

Re: [dpdk-dev] [RFC PATCH 5/5] kni: support multiple userspace process working with kni module

[Ferruh Yigit]

On 5/3/2017 12:21 PM, Hemant Agrawal wrote:
> in case of multiple application using the same KNI module,
> protect that one application will only clean it's own devices.

Idea looks OK, but there is already a check in the module that prevents
/dev/kni opened by more than one process [1], did you already removed
that limitation? Or is this something else?

[1]
kni_open(...) {
...
if (test_and_set_bit(KNI_DEV_IN_USE_BIT_NUM, &knet->device_in_use))
	return -EBUSY;
	...
}

> 
> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
> ---
<...>

Re: [dpdk-dev] [RFC PATCH 5/5] kni: support multiple userspace process working with kni module

[Hemant Agrawal]

On 5/5/2017 6:38 PM, Ferruh Yigit wrote:
> On 5/3/2017 12:21 PM, Hemant Agrawal wrote:
>> in case of multiple application using the same KNI module,
>> protect that one application will only clean it's own devices.
>
> Idea looks OK, but there is already a check in the module that prevents
> /dev/kni opened by more than one process [1], did you already removed
> that limitation? Or is this something else?
>
> [1]
> kni_open(...) {
> ...
> if (test_and_set_bit(KNI_DEV_IN_USE_BIT_NUM, &knet->device_in_use))
> 	return -EBUSY;
> 	...
> }
>

Yes! I have removed that. I will send that in next version.


>>
>> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
>> ---
> <...>
>

Re: [dpdk-dev] [RFC PATCH 1/5] kni: change and configure mac address

[Hemant Agrawal]

On 5/5/2017 4:58 PM, Ferruh Yigit wrote:
> On 5/3/2017 12:21 PM, Hemant Agrawal wrote:
>> This patch adds following:
>> 1. option to configure the mac address during create
>> 2. inform usespace, if mac address is being changed in linux
>>
>> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
>> ---
>>  .../linuxapp/eal/include/exec-env/rte_kni_common.h        |  3 +++
>>  lib/librte_eal/linuxapp/kni/kni_misc.c                    |  6 +++++-
>>  lib/librte_eal/linuxapp/kni/kni_net.c                     | 15 +++++++++++++--
>>  lib/librte_kni/rte_kni.c                                  | 12 ++++++++++++
>>  lib/librte_kni/rte_kni.h                                  |  8 ++++++++
>>  5 files changed, 41 insertions(+), 3 deletions(-)
>>
>> diff --git a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
>> index 2ac879f..e9fdc73 100644
>> --- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
>> +++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
>> @@ -80,6 +80,7 @@ enum rte_kni_req_id {
>>  	RTE_KNI_REQ_UNKNOWN = 0,
>>  	RTE_KNI_REQ_CHANGE_MTU,
>>  	RTE_KNI_REQ_CFG_NETWORK_IF,
>> +	RTE_KNI_REQ_CHANGE_MAC_ADDR,
>>  	RTE_KNI_REQ_MAX,
>>  };
>>
>> @@ -92,6 +93,7 @@ struct rte_kni_request {
>>  	union {
>>  		uint32_t new_mtu;    /**< New MTU */
>>  		uint8_t if_up;       /**< 1: interface up, 0: interface down */
>> +		uint8_t mac_addr[6]; /**< MAC address for interface */
>>  	};
>>  	int32_t result;               /**< Result for processing request */
>>  } __attribute__((__packed__));
>> @@ -168,6 +170,7 @@ struct rte_kni_device_info {
>>
>>  	/* mbuf size */
>>  	unsigned mbuf_size;
>> +	char macaddr[6];              /**< Mac Address */
>
> I think it is good to use same variable name for same reason, above uses
> "mac_addr" here "macaddr", please pick one.
>
> And perhaps field comment can be removed, it is not adding any extra
> information.

ok

>
>>  };
>>
>>  #define KNI_DEVICE "kni"
>> diff --git a/lib/librte_eal/linuxapp/kni/kni_misc.c b/lib/librte_eal/linuxapp/kni/kni_misc.c
>> index 7590f1f..90879fa 100644
>> --- a/lib/librte_eal/linuxapp/kni/kni_misc.c
>> +++ b/lib/librte_eal/linuxapp/kni/kni_misc.c
>> @@ -458,12 +458,16 @@ struct kni_net {
>>
>>  	if (kni->lad_dev)
>>  		ether_addr_copy(net_dev->dev_addr, kni->lad_dev->dev_addr);
>> -	else
>> +	else {
>>  		/*
>>  		 * Generate random mac address. eth_random_addr() is the newer
>>  		 * version of generating mac address in linux kernel.
>>  		 */
>>  		random_ether_addr(net_dev->dev_addr);
>> +		
>> +		/* todo - check if user supplied mac address is available*/
>
> Can we implement todo J

I am just thinking about how to check, if user has provided the mac 
address or he want the random address.

Can we make it mandatory for user to supply the mac address always?
>
>> +		memcpy(net_dev->dev_addr, dev_info.macaddr, ETH_ALEN);
>> +	}
>>
>>  	ret = register_netdev(net_dev);
>>  	if (ret) {
>> diff --git a/lib/librte_eal/linuxapp/kni/kni_net.c b/lib/librte_eal/linuxapp/kni/kni_net.c
>> index db9f489..866cbdd 100644
>> --- a/lib/librte_eal/linuxapp/kni/kni_net.c
>> +++ b/lib/librte_eal/linuxapp/kni/kni_net.c
>> @@ -668,12 +668,23 @@
>>  static int
>>  kni_net_set_mac(struct net_device *netdev, void *p)
>>  {
>> +	int ret;
>> +	struct rte_kni_request req;
>>  	struct sockaddr *addr = p;
>> +	struct kni_dev *kni;
>> +
>> +	kni = netdev_priv(netdev);
>> +	memset(&req, 0, sizeof(req));
>> +	req.req_id = RTE_KNI_REQ_CHANGE_MAC_ADDR;
>>
>> -	if (!is_valid_ether_addr((unsigned char *)(addr->sa_data)))
>> +	if (!is_valid_ether_addr((unsigned char *)(addr->sa_data))) {
>
> Not required to add {
ok
>
>>  		return -EADDRNOTAVAIL;
>> +	}
>> +	memcpy(req.mac_addr, addr->sa_data, netdev->addr_len);
>>  	memcpy(netdev->dev_addr, addr->sa_data, netdev->addr_len);
>> -	return 0;
>> +	ret = kni_net_process_request(kni, &req);
>> +
>> +	return (ret == 0 ? req.result : ret);
>
> If config_mac_address() is not defined, req.result will be undefined.
> You should give initial value to req.

I am doing a memset to req. so req.result will be '0'.

>
>>  }
>>
>>  #ifdef HAVE_CHANGE_CARRIER_CB
>> diff --git a/lib/librte_kni/rte_kni.c b/lib/librte_kni/rte_kni.c
>> index 52fcd4b..d5a717b 100644
>> --- a/lib/librte_kni/rte_kni.c
>> +++ b/lib/librte_kni/rte_kni.c
>> @@ -52,6 +52,10 @@
>>
>>  #define MAX_MBUF_BURST_NUM            32
>>
>> +#ifndef ETH_ADDR_LEN
>> +#define ETH_ADDR_LEN                  6
>> +#endif
>
> This is redundant, rte_kni.h has something similar.
ok
>
>> +
>>  /* Maximum number of ring entries */
>>  #define KNI_FIFO_COUNT_MAX     1024
>>  #define KNI_FIFO_SIZE          (KNI_FIFO_COUNT_MAX * sizeof(void *) + \
>> @@ -368,6 +372,8 @@ struct rte_kni *
>>  	dev_info.group_id = conf->group_id;
>>  	dev_info.mbuf_size = conf->mbuf_size;
>>
>> +	memcpy(dev_info.macaddr, conf->macaddr, ETH_ADDR_LEN);
>> +
>>  	snprintf(ctx->name, RTE_KNI_NAMESIZE, "%s", intf_name);
>>  	snprintf(dev_info.name, RTE_KNI_NAMESIZE, "%s", intf_name);
>>
>> @@ -541,6 +547,11 @@ struct rte_kni *
>>  			req->result = kni->ops.config_network_if(\
>>  					kni->ops.port_id, req->if_up);
>>  		break;
>> +	case RTE_KNI_REQ_CHANGE_MAC_ADDR: /* Change MAC Address */
>> +		if (kni->ops.config_mac_address)
>> +			req->result = kni->ops.config_mac_address(kni->ops.port_id,
>> +							req->mac_addr);
>> +		break;
>>  	default:
>>  		RTE_LOG(ERR, KNI, "Unknown request id %u\n", req->req_id);
>>  		req->result = -EINVAL;
>> @@ -735,6 +746,7 @@ struct rte_kni *
>>
>>  	kni->ops.change_mtu = NULL;
>>  	kni->ops.config_network_if = NULL;
>> +	kni->ops.config_mac_address = NULL;
>
> Can do memset here perhaps?
>
> Also need to update kni_check_request_register(), it only checks
> change_mtu and config_network_if pointers to decide if ops registered,
> since there are more ops now, that logic needs to be updated.
>
ok

>>  	return 0;
>>  }
>>  void
>> diff --git a/lib/librte_kni/rte_kni.h b/lib/librte_kni/rte_kni.h
>> index 37deb47..5d2a233 100644
>> --- a/lib/librte_kni/rte_kni.h
>> +++ b/lib/librte_kni/rte_kni.h
>> @@ -59,6 +59,10 @@
>>  struct rte_kni;
>>  struct rte_mbuf;
>>
>> +#ifndef ETH_ADDR_LEN
>> +#define ETH_ADDR_LEN                  6
>> +#endif
>> +
>
> Can include rte_ether.h, and use ETHER_ADDR_LEN here.
ok

>
>>  /**
>>   * Structure which has the function pointers for KNI interface.
>>   */
>> @@ -70,6 +74,9 @@ struct rte_kni_ops {
>>
>>  	/* Pointer to function of configuring network interface */
>>  	int (*config_network_if)(uint8_t port_id, uint8_t if_up);
>> +
>> +	/* Pointer to function of configuring mac address */
>> +	int (*config_mac_address)(uint8_t port_id, uint8_t mac_addr[6]);
>
> Can replace "6" with ETHER_ADDR_LEN
>
> Two things here:
>
> 1) Providing rte_kni_ops interface during creating a KNI interface links
> KNI interface a backing DPDK port. And kni_ops applied on to this port.
>
> For the case "config_mac_address", it is obvious that what we want to do
> on to DPDK port. Why not provide a default implementation for this
> function in librte_kni (via this patch) and set it by default.
> Application still can overwrite it if it has something else to do. This
> saves effort for others.
>
There are two things:
1. application changing it or initially configuring
2. Change it via Linux - use ifconfig to change it. Application  need to 
implement the ops to get this information, so that it can update it in 
net driver.

> 2) Can you please update sample application with this new kni_ops, again
> in this patch?
>

I will, I want to first get comment on the idea.

>
>>  };
>>
>>  /**
>> @@ -87,6 +94,7 @@ struct rte_kni_conf {
>>  	unsigned mbuf_size; /* mbuf size */
>>  	struct rte_pci_addr addr;
>>  	struct rte_pci_id id;
>> +	char macaddr[ETH_ADDR_LEN]; /* MAC address assigned to KNI */
>>
>>  	__extension__
>>  	uint8_t force_bind : 1; /* Flag to bind kernel thread */
>>
>
>

Re: [dpdk-dev] [RFC PATCH 1/5] kni: change and configure mac address

[Ferruh Yigit]

On 5/3/2017 4:21 AM, Hemant Agrawal wrote:
> This patch adds following:
> 1. option to configure the mac address during create
> 2. inform usespace, if mac address is being changed in linux
> 
> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>

Hi Hemant,

This RFC is waiting in patchwork for a while, is there plan to send a v1? I will
mark patchset as RFC in patchwork.

As far as I can follow latest patchset status is:

provide initial value and add capability to change
1/5: MAC address
2/5: promisc mode
3/5: MTU

There is a change request for 1/5, other two looks OK.
But for all three the action done in kni_ops, which means application needs to
implement these features.
Not sure about pushing these common tasks to application, instead of all
applications implement same thing, does it make sense to implement them in kni
library?

4/5: add gso_size info
Nak because it uses mbuf field with another meaning

5/5: Add cleanup fix for multi process
Multi process support is not supported in KNI at first place, if there is a need
first it needs to be introduced and tested.


Thanks,
ferruh

Re: [dpdk-dev] [RFC PATCH 1/5] kni: change and configure mac address

[Hemant Agrawal]

On 11/29/2017 4:01 AM, Ferruh Yigit wrote:
> On 5/3/2017 4:21 AM, Hemant Agrawal wrote:
>> This patch adds following:
>> 1. option to configure the mac address during create
>> 2. inform usespace, if mac address is being changed in linux
>>
>> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
>
> Hi Hemant,
Hi Ferruh,

Thanks for looking into it. It is a just a co-incidence that I started 
working on these again yesterday :)

I plan to send the updated patch along with application change in next 
few days.

>
> This RFC is waiting in patchwork for a while, is there plan to send a v1? I will
> mark patchset as RFC in patchwork.
>
> As far as I can follow latest patchset status is:
>
> provide initial value and add capability to change
> 1/5: MAC address
> 2/5: promisc mode
> 3/5: MTU
>
> There is a change request for 1/5, other two looks OK.

I am taking care of comments.

> But for all three the action done in kni_ops, which means application needs to
> implement these features.
> Not sure about pushing these common tasks to application, instead of all
> applications implement same thing, does it make sense to implement them in kni
> library?

I don't think it is a good thing to implement them in library itself.
Libary may not know about the equivalent Ethernet port and it may be a 
logical kni port also.

>
> 4/5: add gso_size info
> Nak because it uses mbuf field with another meaning
>
ok, I will try to find other means.

> 5/5: Add cleanup fix for multi process
> Multi process support is not supported in KNI at first place, if there is a need
> first it needs to be introduced and tested.

ok
>
>
> Thanks,
> ferruh
>