DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH v2 0/7] kni: cleanups and improvements
@ 2019-06-10 17:51 Stephen Hemminger
  2019-06-10 17:51 ` [dpdk-dev] [PATCH v2 1/7] kni: don't need stubs for rx_mode or ioctl Stephen Hemminger
                   ` (12 more replies)
  0 siblings, 13 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-10 17:51 UTC (permalink / raw)
  To: ferruh.yigit; +Cc: dev, Stephen Hemminger

While testing KNI with netvsc, saw lots of places more code
could be safely removed from KNI kernel driver.

This is still mostly "putting lipstick on a pig" all users
would be better off using virtio_user rather than KNI.

v2 - get rid of unnecessary padding, combine the unused field patches

Stephen Hemminger (7):
  kni: don't need stubs for rx_mode or ioctl
  kni: use netdev_alloc_skb
  kni: don't keep stats in kni_net
  kni: drop unused fields
  kni: use proper type for kni fifo's
  kni: return -EFAULT if copy_from_user fails
  doc: update KNI documentation

 .../sample_app_ug/kernel_nic_interface.rst    | 18 ++---
 kernel/linux/kni/kni_dev.h                    | 21 ++---
 kernel/linux/kni/kni_misc.c                   | 17 ++--
 kernel/linux/kni/kni_net.c                    | 79 +++++--------------
 4 files changed, 38 insertions(+), 97 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 63+ messages in thread

* [dpdk-dev] [PATCH v2 1/7] kni: don't need stubs for rx_mode or ioctl
  2019-06-10 17:51 [dpdk-dev] [PATCH v2 0/7] kni: cleanups and improvements Stephen Hemminger
@ 2019-06-10 17:51 ` Stephen Hemminger
  2019-06-10 17:51 ` [dpdk-dev] [PATCH v2 2/7] kni: use netdev_alloc_skb Stephen Hemminger
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-10 17:51 UTC (permalink / raw)
  To: ferruh.yigit; +Cc: dev, Stephen Hemminger

The netdev subsystem already handles case where
network sevice does not support ioctl.

If device has no rx_mode hook it is not called.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 kernel/linux/kni/kni_net.c | 19 -------------------
 1 file changed, 19 deletions(-)

diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c
index ad8365877cda..c86337d099ab 100644
--- a/kernel/linux/kni/kni_net.c
+++ b/kernel/linux/kni/kni_net.c
@@ -593,23 +593,6 @@ kni_net_tx_timeout(struct net_device *dev)
 	netif_wake_queue(dev);
 }
 
-/*
- * Ioctl commands
- */
-static int
-kni_net_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
-{
-	pr_debug("kni_net_ioctl group:%d cmd:%d\n",
-		((struct kni_dev *)netdev_priv(dev))->group_id, cmd);
-
-	return -EOPNOTSUPP;
-}
-
-static void
-kni_net_set_rx_mode(struct net_device *dev)
-{
-}
-
 static int
 kni_net_change_mtu(struct net_device *dev, int new_mtu)
 {
@@ -758,8 +741,6 @@ static const struct net_device_ops kni_net_netdev_ops = {
 	.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,
-	.ndo_set_rx_mode = kni_net_set_rx_mode,
 	.ndo_get_stats = kni_net_stats,
 	.ndo_tx_timeout = kni_net_tx_timeout,
 	.ndo_set_mac_address = kni_net_set_mac,
-- 
2.20.1


^ permalink raw reply	[flat|nested] 63+ messages in thread

* [dpdk-dev] [PATCH v2 2/7] kni: use netdev_alloc_skb
  2019-06-10 17:51 [dpdk-dev] [PATCH v2 0/7] kni: cleanups and improvements Stephen Hemminger
  2019-06-10 17:51 ` [dpdk-dev] [PATCH v2 1/7] kni: don't need stubs for rx_mode or ioctl Stephen Hemminger
@ 2019-06-10 17:51 ` Stephen Hemminger
  2019-06-10 17:51 ` [dpdk-dev] [PATCH v2 3/7] kni: don't keep stats in kni_net Stephen Hemminger
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-10 17:51 UTC (permalink / raw)
  To: ferruh.yigit; +Cc: dev, Stephen Hemminger

netdev_alloc_skb is optimized to any alignment or setup
of skb->dev that is required. The kernel intentionall does
not pad packets on x86, because it is faster.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 kernel/linux/kni/kni_net.c | 17 +++--------------
 1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c
index c86337d099ab..cce5e7eb991f 100644
--- a/kernel/linux/kni/kni_net.c
+++ b/kernel/linux/kni/kni_net.c
@@ -340,16 +340,13 @@ kni_net_rx_normal(struct kni_dev *kni)
 		data_kva = kva2data_kva(kva);
 		kni->va[i] = pa2va(kni->pa[i], kva);
 
-		skb = dev_alloc_skb(len + 2);
+		skb = netdev_alloc_skb(dev, len);
 		if (!skb) {
 			/* Update statistics */
 			kni->stats.rx_dropped++;
 			continue;
 		}
 
-		/* Align IP on 16B boundary */
-		skb_reserve(skb, 2);
-
 		if (kva->nb_segs == 1) {
 			memcpy(skb_put(skb, len), data_kva, len);
 		} else {
@@ -368,7 +365,6 @@ kni_net_rx_normal(struct kni_dev *kni)
 			}
 		}
 
-		skb->dev = dev;
 		skb->protocol = eth_type_trans(skb, dev);
 		skb->ip_summed = CHECKSUM_UNNECESSARY;
 
@@ -512,26 +508,20 @@ kni_net_rx_lo_fifo_skb(struct kni_dev *kni)
 		data_kva = kva2data_kva(kva);
 		kni->va[i] = pa2va(kni->pa[i], kva);
 
-		skb = dev_alloc_skb(len + 2);
+		skb = netdev_alloc_skb(dev, len);
 		if (skb) {
-			/* Align IP on 16B boundary */
-			skb_reserve(skb, 2);
 			memcpy(skb_put(skb, len), data_kva, len);
-			skb->dev = dev;
 			skb->ip_summed = CHECKSUM_UNNECESSARY;
 			dev_kfree_skb(skb);
 		}
 
 		/* Simulate real usage, allocate/copy skb twice */
-		skb = dev_alloc_skb(len + 2);
+		skb = netdev_alloc_skb(dev, len);
 		if (skb == NULL) {
 			kni->stats.rx_dropped++;
 			continue;
 		}
 
-		/* Align IP on 16B boundary */
-		skb_reserve(skb, 2);
-
 		if (kva->nb_segs == 1) {
 			memcpy(skb_put(skb, len), data_kva, len);
 		} else {
@@ -550,7 +540,6 @@ kni_net_rx_lo_fifo_skb(struct kni_dev *kni)
 			}
 		}
 
-		skb->dev = dev;
 		skb->ip_summed = CHECKSUM_UNNECESSARY;
 
 		kni->stats.rx_bytes += len;
-- 
2.20.1


^ permalink raw reply	[flat|nested] 63+ messages in thread

* [dpdk-dev] [PATCH v2 3/7] kni: don't keep stats in kni_net
  2019-06-10 17:51 [dpdk-dev] [PATCH v2 0/7] kni: cleanups and improvements Stephen Hemminger
  2019-06-10 17:51 ` [dpdk-dev] [PATCH v2 1/7] kni: don't need stubs for rx_mode or ioctl Stephen Hemminger
  2019-06-10 17:51 ` [dpdk-dev] [PATCH v2 2/7] kni: use netdev_alloc_skb Stephen Hemminger
@ 2019-06-10 17:51 ` Stephen Hemminger
  2019-06-10 17:51 ` [dpdk-dev] [PATCH v2 4/7] kni: drop unused fields Stephen Hemminger
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-10 17:51 UTC (permalink / raw)
  To: ferruh.yigit; +Cc: dev, Stephen Hemminger

Since kernel 2.6.28 the network subsystem has provided
dev->stats for devices to use statistics handling and is the
default if no ndo_get_stats is provided.

This allow allows for 64 bit (rather than just 32 bit)
statistics with KNI.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 kernel/linux/kni/kni_dev.h |  1 -
 kernel/linux/kni/kni_net.c | 43 +++++++++++++-------------------------
 2 files changed, 15 insertions(+), 29 deletions(-)

diff --git a/kernel/linux/kni/kni_dev.h b/kernel/linux/kni/kni_dev.h
index 44a5a61f7279..f3a1f60d4bdc 100644
--- a/kernel/linux/kni/kni_dev.h
+++ b/kernel/linux/kni/kni_dev.h
@@ -39,7 +39,6 @@ struct kni_dev {
 	/* kni list */
 	struct list_head list;
 
-	struct net_device_stats stats;
 	int status;
 	uint16_t group_id;           /* Group ID of a group of KNI devices */
 	uint32_t core_id;            /* Core ID to bind */
diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c
index cce5e7eb991f..06310fec57bb 100644
--- a/kernel/linux/kni/kni_net.c
+++ b/kernel/linux/kni/kni_net.c
@@ -291,15 +291,15 @@ kni_net_tx(struct sk_buff *skb, struct net_device *dev)
 
 	/* Free skb and update statistics */
 	dev_kfree_skb(skb);
-	kni->stats.tx_bytes += len;
-	kni->stats.tx_packets++;
+	dev->stats.tx_bytes += len;
+	dev->stats.tx_packets++;
 
 	return NETDEV_TX_OK;
 
 drop:
 	/* Free skb and update statistics */
 	dev_kfree_skb(skb);
-	kni->stats.tx_dropped++;
+	dev->stats.tx_dropped++;
 
 	return NETDEV_TX_OK;
 }
@@ -343,7 +343,7 @@ kni_net_rx_normal(struct kni_dev *kni)
 		skb = netdev_alloc_skb(dev, len);
 		if (!skb) {
 			/* Update statistics */
-			kni->stats.rx_dropped++;
+			dev->stats.rx_dropped++;
 			continue;
 		}
 
@@ -372,8 +372,8 @@ kni_net_rx_normal(struct kni_dev *kni)
 		netif_rx_ni(skb);
 
 		/* Update statistics */
-		kni->stats.rx_bytes += len;
-		kni->stats.rx_packets++;
+		dev->stats.rx_bytes += len;
+		dev->stats.rx_packets++;
 	}
 
 	/* Burst enqueue mbufs into free_q */
@@ -396,6 +396,7 @@ kni_net_rx_lo_fifo(struct kni_dev *kni)
 	void *data_kva;
 	struct rte_kni_mbuf *alloc_kva;
 	void *alloc_data_kva;
+	struct net_device *dev = kni->net_dev;
 
 	/* Get the number of entries in rx_q */
 	num_rq = kni_fifo_count(kni->rx_q);
@@ -443,8 +444,8 @@ kni_net_rx_lo_fifo(struct kni_dev *kni)
 			alloc_kva->pkt_len = len;
 			alloc_kva->data_len = len;
 
-			kni->stats.tx_bytes += len;
-			kni->stats.rx_bytes += len;
+			dev->stats.tx_bytes += len;
+			dev->stats.rx_bytes += len;
 		}
 
 		/* Burst enqueue mbufs into tx_q */
@@ -464,8 +465,8 @@ kni_net_rx_lo_fifo(struct kni_dev *kni)
 	 * Update statistic, and enqueue/dequeue failure is impossible,
 	 * as all queues are checked at first.
 	 */
-	kni->stats.tx_packets += num;
-	kni->stats.rx_packets += num;
+	dev->stats.tx_packets += num;
+	dev->stats.rx_packets += num;
 }
 
 /*
@@ -518,7 +519,7 @@ kni_net_rx_lo_fifo_skb(struct kni_dev *kni)
 		/* Simulate real usage, allocate/copy skb twice */
 		skb = netdev_alloc_skb(dev, len);
 		if (skb == NULL) {
-			kni->stats.rx_dropped++;
+			dev->stats.rx_dropped++;
 			continue;
 		}
 
@@ -542,8 +543,8 @@ kni_net_rx_lo_fifo_skb(struct kni_dev *kni)
 
 		skb->ip_summed = CHECKSUM_UNNECESSARY;
 
-		kni->stats.rx_bytes += len;
-		kni->stats.rx_packets++;
+		dev->stats.rx_bytes += len;
+		dev->stats.rx_packets++;
 
 		/* call tx interface */
 		kni_net_tx(skb, dev);
@@ -573,12 +574,10 @@ kni_net_rx(struct kni_dev *kni)
 static void
 kni_net_tx_timeout(struct net_device *dev)
 {
-	struct kni_dev *kni = netdev_priv(dev);
-
 	pr_debug("Transmit timeout at %ld, latency %ld\n", jiffies,
 			jiffies - dev_trans_start(dev));
 
-	kni->stats.tx_errors++;
+	dev->stats.tx_errors++;
 	netif_wake_queue(dev);
 }
 
@@ -627,17 +626,6 @@ kni_net_poll_resp(struct kni_dev *kni)
 		wake_up_interruptible(&kni->wq);
 }
 
-/*
- * Return statistics to the caller
- */
-static struct net_device_stats *
-kni_net_stats(struct net_device *dev)
-{
-	struct kni_dev *kni = netdev_priv(dev);
-
-	return &kni->stats;
-}
-
 /*
  *  Fill the eth header
  */
@@ -730,7 +718,6 @@ static const struct net_device_ops kni_net_netdev_ops = {
 	.ndo_change_rx_flags = kni_net_set_promiscusity,
 	.ndo_start_xmit = kni_net_tx,
 	.ndo_change_mtu = kni_net_change_mtu,
-	.ndo_get_stats = kni_net_stats,
 	.ndo_tx_timeout = kni_net_tx_timeout,
 	.ndo_set_mac_address = kni_net_set_mac,
 #ifdef HAVE_CHANGE_CARRIER_CB
-- 
2.20.1


^ permalink raw reply	[flat|nested] 63+ messages in thread

* [dpdk-dev] [PATCH v2 4/7] kni: drop unused fields
  2019-06-10 17:51 [dpdk-dev] [PATCH v2 0/7] kni: cleanups and improvements Stephen Hemminger
                   ` (2 preceding siblings ...)
  2019-06-10 17:51 ` [dpdk-dev] [PATCH v2 3/7] kni: don't keep stats in kni_net Stephen Hemminger
@ 2019-06-10 17:51 ` Stephen Hemminger
  2019-06-10 17:51 ` [dpdk-dev] [PATCH v2 5/7] kni: use proper type for kni fifo's Stephen Hemminger
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-10 17:51 UTC (permalink / raw)
  To: ferruh.yigit; +Cc: dev, Stephen Hemminger

The kni net structure only exists in driver no API/ABI.
Several fields were either totally unused or set and never used.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 kernel/linux/kni/kni_dev.h  | 8 --------
 kernel/linux/kni/kni_misc.c | 1 -
 2 files changed, 9 deletions(-)

diff --git a/kernel/linux/kni/kni_dev.h b/kernel/linux/kni/kni_dev.h
index f3a1f60d4bdc..f3e6c3ca4efa 100644
--- a/kernel/linux/kni/kni_dev.h
+++ b/kernel/linux/kni/kni_dev.h
@@ -39,8 +39,6 @@ struct kni_dev {
 	/* kni list */
 	struct list_head list;
 
-	int status;
-	uint16_t group_id;           /* Group ID of a group of KNI devices */
 	uint32_t core_id;            /* Core ID to bind */
 	char name[RTE_KNI_NAMESIZE]; /* Network device name */
 	struct task_struct *pthread;
@@ -49,9 +47,6 @@ struct kni_dev {
 	wait_queue_head_t wq;
 	struct mutex sync_lock;
 
-	/* PCI device id */
-	uint16_t device_id;
-
 	/* kni device */
 	struct net_device *net_dev;
 
@@ -82,9 +77,6 @@ struct kni_dev {
 	/* mbuf size */
 	uint32_t mbuf_size;
 
-	/* synchro for request processing */
-	unsigned long synchro;
-
 	/* buffers */
 	void *pa[MBUF_BURST_SZ];
 	void *va[MBUF_BURST_SZ];
diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c
index af18c67c422f..6a206d883c0d 100644
--- a/kernel/linux/kni/kni_misc.c
+++ b/kernel/linux/kni/kni_misc.c
@@ -346,7 +346,6 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num,
 	kni = netdev_priv(net_dev);
 
 	kni->net_dev = net_dev;
-	kni->group_id = dev_info.group_id;
 	kni->core_id = dev_info.core_id;
 	strncpy(kni->name, dev_info.name, RTE_KNI_NAMESIZE);
 
-- 
2.20.1


^ permalink raw reply	[flat|nested] 63+ messages in thread

* [dpdk-dev] [PATCH v2 5/7] kni: use proper type for kni fifo's
  2019-06-10 17:51 [dpdk-dev] [PATCH v2 0/7] kni: cleanups and improvements Stephen Hemminger
                   ` (3 preceding siblings ...)
  2019-06-10 17:51 ` [dpdk-dev] [PATCH v2 4/7] kni: drop unused fields Stephen Hemminger
@ 2019-06-10 17:51 ` Stephen Hemminger
  2019-06-10 17:51 ` [dpdk-dev] [PATCH v2 6/7] kni: return -EFAULT if copy_from_user fails Stephen Hemminger
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-10 17:51 UTC (permalink / raw)
  To: ferruh.yigit; +Cc: dev, Stephen Hemminger

Using void * instead of proper type is unsafe practice.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 kernel/linux/kni/kni_dev.h | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/kernel/linux/kni/kni_dev.h b/kernel/linux/kni/kni_dev.h
index f3e6c3ca4efa..ceba5f73c1d9 100644
--- a/kernel/linux/kni/kni_dev.h
+++ b/kernel/linux/kni/kni_dev.h
@@ -51,22 +51,22 @@ struct kni_dev {
 	struct net_device *net_dev;
 
 	/* queue for packets to be sent out */
-	void *tx_q;
+	struct rte_kni_fifo *tx_q;
 
 	/* queue for the packets received */
-	void *rx_q;
+	struct rte_kni_fifo *rx_q;
 
 	/* queue for the allocated mbufs those can be used to save sk buffs */
-	void *alloc_q;
+	struct rte_kni_fifo *alloc_q;
 
 	/* free queue for the mbufs to be freed */
-	void *free_q;
+	struct rte_kni_fifo *free_q;
 
 	/* request queue */
-	void *req_q;
+	struct rte_kni_fifo *req_q;
 
 	/* response queue */
-	void *resp_q;
+	struct rte_kni_fifo *resp_q;
 
 	void *sync_kva;
 	void *sync_va;
-- 
2.20.1


^ permalink raw reply	[flat|nested] 63+ messages in thread

* [dpdk-dev] [PATCH v2 6/7] kni: return -EFAULT if copy_from_user fails
  2019-06-10 17:51 [dpdk-dev] [PATCH v2 0/7] kni: cleanups and improvements Stephen Hemminger
                   ` (4 preceding siblings ...)
  2019-06-10 17:51 ` [dpdk-dev] [PATCH v2 5/7] kni: use proper type for kni fifo's Stephen Hemminger
@ 2019-06-10 17:51 ` Stephen Hemminger
  2019-06-10 17:51 ` [dpdk-dev] [PATCH v2 7/7] doc: update KNI documentation Stephen Hemminger
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-10 17:51 UTC (permalink / raw)
  To: ferruh.yigit; +Cc: dev, Stephen Hemminger

The correct thing to return if user gives a bad data
is to return -EFAULT. Logging is also discouraged because
it could be used as a DoS attack.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 kernel/linux/kni/kni_misc.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c
index 6a206d883c0d..a67991066cd9 100644
--- a/kernel/linux/kni/kni_misc.c
+++ b/kernel/linux/kni/kni_misc.c
@@ -301,11 +301,8 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num,
 		return -EINVAL;
 
 	/* Copy kni info from user space */
-	ret = copy_from_user(&dev_info, (void *)ioctl_param, sizeof(dev_info));
-	if (ret) {
-		pr_err("copy_from_user in kni_ioctl_create");
-		return -EIO;
-	}
+	if (copy_from_user(&dev_info, (void *)ioctl_param, sizeof(dev_info)))
+		return -EFAULT;
 
 	/* Check if name is zero-ended */
 	if (strnlen(dev_info.name, sizeof(dev_info.name)) == sizeof(dev_info.name)) {
@@ -433,15 +430,12 @@ kni_ioctl_release(struct net *net, uint32_t ioctl_num,
 	if (_IOC_SIZE(ioctl_num) > sizeof(dev_info))
 		return -EINVAL;
 
-	ret = copy_from_user(&dev_info, (void *)ioctl_param, sizeof(dev_info));
-	if (ret) {
-		pr_err("copy_from_user in kni_ioctl_release");
-		return -EIO;
-	}
+	if (copy_from_user(&dev_info, (void *)ioctl_param, sizeof(dev_info)))
+		return -EFAULT;
 
 	/* Release the network device according to its name */
 	if (strlen(dev_info.name) == 0)
-		return ret;
+		return -EINVAL;
 
 	down_write(&knet->kni_list_lock);
 	list_for_each_entry_safe(dev, n, &knet->kni_list_head, list) {
-- 
2.20.1


^ permalink raw reply	[flat|nested] 63+ messages in thread

* [dpdk-dev] [PATCH v2 7/7] doc: update KNI documentation
  2019-06-10 17:51 [dpdk-dev] [PATCH v2 0/7] kni: cleanups and improvements Stephen Hemminger
                   ` (5 preceding siblings ...)
  2019-06-10 17:51 ` [dpdk-dev] [PATCH v2 6/7] kni: return -EFAULT if copy_from_user fails Stephen Hemminger
@ 2019-06-10 17:51 ` Stephen Hemminger
  2019-06-11 20:54 ` [dpdk-dev] [PATCH v2 0/7] kni: cleanups and improvements Stephen Hemminger
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-10 17:51 UTC (permalink / raw)
  To: ferruh.yigit; +Cc: dev, Stephen Hemminger

Make the KNI documentation reflect modern kernel networking.
Ifconfig has been superseded by iproute2 for 15 years.
Iproute2 is well maintained, supports current feature set.

Ethtool is no longer supported by KNI.
Tshark is a better replacement for tcpdump.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 .../sample_app_ug/kernel_nic_interface.rst     | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/doc/guides/sample_app_ug/kernel_nic_interface.rst b/doc/guides/sample_app_ug/kernel_nic_interface.rst
index a7e549d5213a..422bd8c98465 100644
--- a/doc/guides/sample_app_ug/kernel_nic_interface.rst
+++ b/doc/guides/sample_app_ug/kernel_nic_interface.rst
@@ -21,14 +21,14 @@ The FIFO queues contain pointers to data packets in the DPDK. This:
 
 *   Provides a faster mechanism to interface with the kernel net stack and eliminates system calls
 
-*   Facilitates the DPDK using standard Linux* userspace net tools (tcpdump, ftp, and so on)
+*   Facilitates the DPDK using standard Linux* userspace net tools (tshark, rsync, and so on)
 
 *   Eliminate the copy_to_user and copy_from_user operations on packets.
 
 The Kernel NIC Interface sample application is a simple example that demonstrates the use
 of the DPDK to create a path for packets to go through the Linux* kernel.
 This is done by creating one or more kernel net devices for each of the DPDK ports.
-The application allows the use of standard Linux tools (ethtool, ifconfig, tcpdump) with the DPDK ports and
+The application allows the use of standard Linux tools (iproute, tshark) with the DPDK ports and
 also the exchange of packets between the DPDK application and the Linux* kernel.
 
 The Kernel NIC Interface sample application requires that the
@@ -220,13 +220,13 @@ Enable KNI interface and assign an IP address:
 
 .. code-block:: console
 
-    # ifconfig vEth0_0 192.168.0.1
+    # ip addr add dev vEth0_0 192.168.0.1
 
 Show KNI interface configuration and statistics:
 
 .. code-block:: console
 
-    # ifconfig vEth0_0
+    # ip -s -d addr show vEth0_0
 
 The user can also check and reset the packet statistics inside the ``kni``
 application by sending the app the USR1 and USR2 signals:
@@ -234,16 +234,16 @@ application by sending the app the USR1 and USR2 signals:
 .. code-block:: console
 
     # Print statistics
-    # kill -SIGUSR1 `pidof kni`
+    # pkill -USR1 kni
 
     # Zero statistics
-    # kill -SIGUSR2 `pidof kni`
+    # pkill -USR2 kni
 
 Dump network traffic:
 
 .. code-block:: console
 
-    # tcpdump -i vEth0_0
+    # tshark -n -i vEth0_0
 
 The normal Linux commands can also be used to change the MAC address and
 MTU size used by the physical NIC which corresponds to the KNI interface.
@@ -254,13 +254,13 @@ Change the MAC address:
 
 .. code-block:: console
 
-    # ifconfig vEth0_0 hw ether 0C:01:02:03:04:08
+    # ip link set dev vEth0_0 lladdr 0C:01:02:03:04:08
 
 Change the MTU size:
 
 .. code-block:: console
 
-    # ifconfig vEth0_0 mtu 1450
+    # ip link set dev vEth0_0 mtu 1450
 
 When the ``kni`` application is closed, all the KNI interfaces are deleted
 from the Linux kernel.
-- 
2.20.1


^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [dpdk-dev] [PATCH v2 0/7] kni: cleanups and improvements
  2019-06-10 17:51 [dpdk-dev] [PATCH v2 0/7] kni: cleanups and improvements Stephen Hemminger
                   ` (6 preceding siblings ...)
  2019-06-10 17:51 ` [dpdk-dev] [PATCH v2 7/7] doc: update KNI documentation Stephen Hemminger
@ 2019-06-11 20:54 ` Stephen Hemminger
  2019-06-11 21:18   ` Lance Richardson
  2019-06-18 16:16 ` [dpdk-dev] [PATCH v3 0/8] kni: fixes and cleanups Stephen Hemminger
                   ` (4 subsequent siblings)
  12 siblings, 1 reply; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-11 20:54 UTC (permalink / raw)
  To: ferruh.yigit; +Cc: dev

On Mon, 10 Jun 2019 10:51:48 -0700
Stephen Hemminger <stephen@networkplumber.org> wrote:

> While testing KNI with netvsc, saw lots of places more code
> could be safely removed from KNI kernel driver.
> 
> This is still mostly "putting lipstick on a pig" all users
> would be better off using virtio_user rather than KNI.
> 
> v2 - get rid of unnecessary padding, combine the unused field patches
> 
> Stephen Hemminger (7):
>   kni: don't need stubs for rx_mode or ioctl
>   kni: use netdev_alloc_skb
>   kni: don't keep stats in kni_net
>   kni: drop unused fields
>   kni: use proper type for kni fifo's
>   kni: return -EFAULT if copy_from_user fails
>   doc: update KNI documentation
> 
>  .../sample_app_ug/kernel_nic_interface.rst    | 18 ++---
>  kernel/linux/kni/kni_dev.h                    | 21 ++---
>  kernel/linux/kni/kni_misc.c                   | 17 ++--
>  kernel/linux/kni/kni_net.c                    | 79 +++++--------------
>  4 files changed, 38 insertions(+), 97 deletions(-)
> 

Don't believe patchwork the patch is fine, it is getting falsely blamed
for failures caused by other changes in ice, bnxt which fail
FreeBSD build.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [dpdk-dev] [PATCH v2 0/7] kni: cleanups and improvements
  2019-06-11 20:54 ` [dpdk-dev] [PATCH v2 0/7] kni: cleanups and improvements Stephen Hemminger
@ 2019-06-11 21:18   ` Lance Richardson
  2019-06-11 21:30     ` Stephen Hemminger
  0 siblings, 1 reply; 63+ messages in thread
From: Lance Richardson @ 2019-06-11 21:18 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Ferruh Yigit, dev

On Tue, Jun 11, 2019 at 4:54 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Mon, 10 Jun 2019 10:51:48 -0700
> Stephen Hemminger <stephen@networkplumber.org> wrote:
>
> > While testing KNI with netvsc, saw lots of places more code
> > could be safely removed from KNI kernel driver.
> >
> > This is still mostly "putting lipstick on a pig" all users
> > would be better off using virtio_user rather than KNI.
> >
> > v2 - get rid of unnecessary padding, combine the unused field patches
> >
> > Stephen Hemminger (7):
> >   kni: don't need stubs for rx_mode or ioctl
> >   kni: use netdev_alloc_skb
> >   kni: don't keep stats in kni_net
> >   kni: drop unused fields
> >   kni: use proper type for kni fifo's
> >   kni: return -EFAULT if copy_from_user fails
> >   doc: update KNI documentation
> >
> >  .../sample_app_ug/kernel_nic_interface.rst    | 18 ++---
> >  kernel/linux/kni/kni_dev.h                    | 21 ++---
> >  kernel/linux/kni/kni_misc.c                   | 17 ++--
> >  kernel/linux/kni/kni_net.c                    | 79 +++++--------------
> >  4 files changed, 38 insertions(+), 97 deletions(-)
> >
>
> Don't believe patchwork the patch is fine, it is getting falsely blamed
> for failures caused by other changes in ice, bnxt which fail
> FreeBSD build.

Do you mean failures like the ones below? If so, I think think they
might be an unintended
consequence of commit a385972c3 "mk: disable warning for packed member pointer".

    Lance

OS: FreeBSD12-64
Target: x86_64-native-bsdapp-gcc
  CC ice_rxtx.o
  CC ice_ethdev.o
cc1: error: unrecognized command line option
'-Wno-address-of-packed-member' [-Werror]
cc1: all warnings being treated as errors
gmake[6]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc/e6c3bc062b1045128114bdf0cfdd236b/dpdk/mk/internal/rte.compile-pre.mk:114:
bnxt_ethdev.o] Error 1
gmake[5]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc/e6c3bc062b1045128114bdf0cfdd236b/dpdk/mk/rte.subdir.mk:37:
bnxt] Error 2
gmake[5]: *** Waiting for unfinished jobs....

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [dpdk-dev] [PATCH v2 0/7] kni: cleanups and improvements
  2019-06-11 21:18   ` Lance Richardson
@ 2019-06-11 21:30     ` Stephen Hemminger
  2019-06-11 21:49       ` Lance Richardson
  0 siblings, 1 reply; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-11 21:30 UTC (permalink / raw)
  To: Lance Richardson; +Cc: Ferruh Yigit, dev

On Tue, 11 Jun 2019 17:18:42 -0400
Lance Richardson <lance.richardson@broadcom.com> wrote:

> On Tue, Jun 11, 2019 at 4:54 PM Stephen Hemminger
> <stephen@networkplumber.org> wrote:
> >
> > On Mon, 10 Jun 2019 10:51:48 -0700
> > Stephen Hemminger <stephen@networkplumber.org> wrote:
> >  
> > > While testing KNI with netvsc, saw lots of places more code
> > > could be safely removed from KNI kernel driver.
> > >
> > > This is still mostly "putting lipstick on a pig" all users
> > > would be better off using virtio_user rather than KNI.
> > >
> > > v2 - get rid of unnecessary padding, combine the unused field patches
> > >
> > > Stephen Hemminger (7):
> > >   kni: don't need stubs for rx_mode or ioctl
> > >   kni: use netdev_alloc_skb
> > >   kni: don't keep stats in kni_net
> > >   kni: drop unused fields
> > >   kni: use proper type for kni fifo's
> > >   kni: return -EFAULT if copy_from_user fails
> > >   doc: update KNI documentation
> > >
> > >  .../sample_app_ug/kernel_nic_interface.rst    | 18 ++---
> > >  kernel/linux/kni/kni_dev.h                    | 21 ++---
> > >  kernel/linux/kni/kni_misc.c                   | 17 ++--
> > >  kernel/linux/kni/kni_net.c                    | 79 +++++--------------
> > >  4 files changed, 38 insertions(+), 97 deletions(-)
> > >  
> >
> > Don't believe patchwork the patch is fine, it is getting falsely blamed
> > for failures caused by other changes in ice, bnxt which fail
> > FreeBSD build.  
> 
> Do you mean failures like the ones below? If so, I think think they
> might be an unintended
> consequence of commit a385972c3 "mk: disable warning for packed member pointer".
> 
>     Lance
> 
> OS: FreeBSD12-64
> Target: x86_64-native-bsdapp-gcc
>   CC ice_rxtx.o
>   CC ice_ethdev.o
> cc1: error: unrecognized command line option
> '-Wno-address-of-packed-member' [-Werror]
> cc1: all warnings being treated as errors
> gmake[6]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc/e6c3bc062b1045128114bdf0cfdd236b/dpdk/mk/internal/rte.compile-pre.mk:114:
> bnxt_ethdev.o] Error 1
> gmake[5]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc/e6c3bc062b1045128114bdf0cfdd236b/dpdk/mk/rte.subdir.mk:37:
> bnxt] Error 2
> gmake[5]: *** Waiting for unfinished jobs....


More than just that

*Build Failed Build #1:
OS: FreeBSD12-64
Target: x86_64-native-bsdapp-gcc+debug
  CC ipn3ke_flow.o
  CC lio_23xx_vf.o
cc1: error: unrecognized command line option '-Wno-address-of-packed-member' [-Werror]
cc1: all warnings being treated as errors
gmake[6]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc+debug/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/internal/rte.compile-pre.mk:116: bnxt_ethdev.o] Error 1
gmake[5]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc+debug/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.subdir.mk:37: bnxt] Error 2
gmake[5]: *** Waiting for unfinished jobs....
  CC i40e_flow.o
  CC rte_eth_null.o
--
  LD ixgbe_ethdev.o
  AR librte_pmd_ixgbe.a
  INSTALL-LIB librte_pmd_ixgbe.a
gmake[4]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc+debug/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.subdir.mk:35: net] Error 2
gmake[3]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc+debug/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.sdkbuild.mk:46: drivers] Error 2
gmake[2]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc+debug/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.sdkroot.mk:99: all] Error 2
gmake[1]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc+debug/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.sdkinstall.mk:61: pre_install] Error 2
gmake: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc+debug/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.sdkroot.mk:77: install] Error 2

*Build Failed Build #2:
OS: FreeBSD12-64
Target: x86_64-native-bsdapp-gcc+shared
  CC rte_eth_null.o.pmd.o
  LD rte_eth_null.o
cc1: error: unrecognized command line option '-Wno-address-of-packed-member' [-Werror]
cc1: all warnings being treated as errors
gmake[6]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc+shared/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/internal/rte.compile-pre.mk:116: bnxt_ethdev.o] Error 1
gmake[6]: *** Waiting for unfinished jobs....
  CC ice_rxtx.o
== Build drivers/net/ring
--
  LD ice_ethdev.o
  CC ice_rxtx_vec_sse.o
  CC octeontx_pkovf.o
gmake[5]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc+shared/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.subdir.mk:35: bnxt] Error 2
gmake[5]: *** Waiting for unfinished jobs....
  CC i40e_tm.o
  CC ice_rxtx_vec_avx2.o
--
  INSTALL-LIB librte_pmd_ixgbe.so.2.1
  LD librte_pmd_qede.so.1.1
  INSTALL-LIB librte_pmd_qede.so.1.1
gmake[4]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc+shared/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.subdir.mk:35: net] Error 2
gmake[3]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc+shared/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.sdkbuild.mk:46: drivers] Error 2
gmake[2]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc+shared/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.sdkroot.mk:99: all] Error 2
gmake[1]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc+shared/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.sdkinstall.mk:61: pre_install] Error 2
gmake: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc+shared/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.sdkroot.mk:77: install] Error 2

*Build Failed Build #3:
OS: FreeBSD12-64
Target: x86_64-native-bsdapp-gcc
  CC ice_dcb.o
  LD rte_eth_null.o
cc1: error: unrecognized command line option '-Wno-address-of-packed-member' [-Werror]
cc1: all warnings being treated as errors
gmake[6]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/internal/rte.compile-pre.mk:114: bnxt_ethdev.o] Error 1
gmake[5]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.subdir.mk:37: bnxt] Error 2
gmake[5]: *** Waiting for unfinished jobs....
  CC ixgbe_phy.o
  CC octeontx_rxtx.o
--
  LD qede_ethdev.o
  AR librte_pmd_qede.a
  INSTALL-LIB librte_pmd_qede.a
gmake[4]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.subdir.mk:35: net] Error 2
gmake[3]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.sdkbuild.mk:46: drivers] Error 2
gmake[2]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.sdkroot.mk:99: all] Error 2
gmake[1]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.sdkinstall.mk:61: pre_install] Error 2
gmake: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.sdkroot.mk:77: install] Error 2

*Build Failed Build #4:
OS: FreeBSD12-64
Target: x86_64-native-bsdapp-clang
#define roundup(x, y)   ((((x)+((y)-1))/(y))*(y))  /* to any y */
        ^
1 error generated.
gmake[6]: *** [/tmp/FreeBSD12-64_K19.02_Clang6.0.1/x86_64-native-bsdapp-clang/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/internal/rte.compile-pre.mk:114: bnxt_ethdev.o] Error 1
  CC ice_flex_pipe.o
gmake[5]: *** [/tmp/FreeBSD12-64_K19.02_Clang6.0.1/x86_64-native-bsdapp-clang/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.subdir.mk:35: bnxt] Error 2
gmake[5]: *** Waiting for unfinished jobs....
  CC lio_mbox.o
  AR librte_pmd_null.a
--
  LD ixgbe_ethdev.o
  AR librte_pmd_ixgbe.a
  INSTALL-LIB librte_pmd_ixgbe.a
gmake[4]: *** [/tmp/FreeBSD12-64_K19.02_Clang6.0.1/x86_64-native-bsdapp-clang/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.subdir.mk:35: net] Error 2
gmake[3]: *** [/tmp/FreeBSD12-64_K19.02_Clang6.0.1/x86_64-native-bsdapp-clang/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.sdkbuild.mk:46: drivers] Error 2
gmake[2]: *** [/tmp/FreeBSD12-64_K19.02_Clang6.0.1/x86_64-native-bsdapp-clang/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.sdkroot.mk:99: all] Error 2
gmake[1]: *** [/tmp/FreeBSD12-64_K19.02_Clang6.0.1/x86_64-native-bsdapp-clang/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.sdkinstall.mk:61: pre_install] Error 2
gmake: *** [/tmp/FreeBSD12-64_K19.02_Clang6.0.1/x86_64-native-bsdapp-clang/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.sdkroot.mk:77: install] Error 2
*Meson Failed Build #1:
OS: UB1604-32
Target:build-gcc-static
FAILED: examples/c590b3c@@dpdk-rxtx_callbacks at exe/rxtx_callbacks_main.c.o 
gcc -Iexamples/c590b3c@@dpdk-rxtx_callbacks at exe -Iexamples -I../examples -Iexamples/rxtx_callbacks -I../examples/rxtx_callbacks -I. -I../ -Iconfig -I../config -Ilib/librte_eal/common/include -I../lib/librte_eal/common/include -I../lib/librte_eal/linux/eal/include -Ilib/librte_eal/common -I../lib/librte_eal/common -Ilib/librte_eal/common/include/arch/x86 -I../lib/librte_eal/common/include/arch/x86 -Ilib/librte_eal -I../lib/librte_eal -Ilib/librte_kvargs -I../lib/librte_kvargs -Ilib/librte_mempool -I../lib/librte_mempool -Ilib/librte_ring -I../lib/librte_ring -Ilib/librte_net -I../lib/librte_net -Ilib/librte_mbuf -I../lib/librte_mbuf -Ilib/librte_ethdev -I../lib/librte_ethdev -Ilib/librte_cmdline -I../lib/librte_cmdline -Ilib/librte_meter -I../lib/librte_meter -fdiagnostics-color=always -pipe -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Werror -O3 -include rte_config.h -Wunused-parameter -Wsign-compare -Wcast-qual -Wno-pointer-to-int-cast -march=native -D_GNU_SOURCE -DALLOW_EXPERIMENTAL_API -MD -MQ 'examples/c590b3c@@dpdk-rxtx_callbacks at exe/rxtx_callbacks_main.c.o' -MF 'examples/c590b3c@@dpdk-rxtx_callbacks at exe/rxtx_callbacks_main.c.o.d' -o 'examples/c590b3c@@dpdk-rxtx_callbacks at exe/rxtx_callbacks_main.c.o' -c ../examples/rxtx_callbacks/main.c
../examples/rxtx_callbacks/main.c: In function ‘port_init’:
../examples/rxtx_callbacks/main.c:172:10: error: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 2 has type ‘uint64_t {aka long long unsigned int}’ [-Werror=format=]
   printf("TSC Freq ~= %lu\nHW Freq ~= %lu\nRatio : %f\n",
          ^
../examples/rxtx_callbacks/main.c:172:10: error: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 3 has type ‘uint64_t {aka long long unsigned int}’ [-Werror=format=]
cc1: all warnings being treated as errors
[1531/1549] Linking target examples/dpdk-vdpa.
[1532/1549] Linking target examples/dpdk-timer.
[1533/1549] Linking target examples/dpdk-tep_termination.
[1534/1549] Compiling C object 'examples/c590b3c@@dpdk-vhost_crypto at exe/vhost_crypto_main.c.o'.
[1535/1549] Compiling C object 'examples/c590b3c@@dpdk-vhost at exe/vhost_main.c.o'.
ninja: build stopped: subcommand failed




^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [dpdk-dev] [PATCH v2 0/7] kni: cleanups and improvements
  2019-06-11 21:30     ` Stephen Hemminger
@ 2019-06-11 21:49       ` Lance Richardson
  0 siblings, 0 replies; 63+ messages in thread
From: Lance Richardson @ 2019-06-11 21:49 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Ferruh Yigit, dev

On Tue, Jun 11, 2019 at 5:30 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Tue, 11 Jun 2019 17:18:42 -0400
> Lance Richardson <lance.richardson@broadcom.com> wrote:
>
> > On Tue, Jun 11, 2019 at 4:54 PM Stephen Hemminger
> > <stephen@networkplumber.org> wrote:
> > >
> > > On Mon, 10 Jun 2019 10:51:48 -0700
> > > Stephen Hemminger <stephen@networkplumber.org> wrote:
> > >
> > > > While testing KNI with netvsc, saw lots of places more code
> > > > could be safely removed from KNI kernel driver.
> > > >
> > > > This is still mostly "putting lipstick on a pig" all users
> > > > would be better off using virtio_user rather than KNI.
> > > >
> > > > v2 - get rid of unnecessary padding, combine the unused field patches
> > > >
> > > > Stephen Hemminger (7):
> > > >   kni: don't need stubs for rx_mode or ioctl
> > > >   kni: use netdev_alloc_skb
> > > >   kni: don't keep stats in kni_net
> > > >   kni: drop unused fields
> > > >   kni: use proper type for kni fifo's
> > > >   kni: return -EFAULT if copy_from_user fails
> > > >   doc: update KNI documentation
> > > >
> > > >  .../sample_app_ug/kernel_nic_interface.rst    | 18 ++---
> > > >  kernel/linux/kni/kni_dev.h                    | 21 ++---
> > > >  kernel/linux/kni/kni_misc.c                   | 17 ++--
> > > >  kernel/linux/kni/kni_net.c                    | 79 +++++--------------
> > > >  4 files changed, 38 insertions(+), 97 deletions(-)
> > > >
> > >
> > > Don't believe patchwork the patch is fine, it is getting falsely blamed
> > > for failures caused by other changes in ice, bnxt which fail
> > > FreeBSD build.
> >
> > Do you mean failures like the ones below? If so, I think think they
> > might be an unintended
> > consequence of commit a385972c3 "mk: disable warning for packed member pointer".
> >
> >     Lance
> >
> > OS: FreeBSD12-64
> > Target: x86_64-native-bsdapp-gcc
> >   CC ice_rxtx.o
> >   CC ice_ethdev.o
> > cc1: error: unrecognized command line option
> > '-Wno-address-of-packed-member' [-Werror]
> > cc1: all warnings being treated as errors
> > gmake[6]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc/e6c3bc062b1045128114bdf0cfdd236b/dpdk/mk/internal/rte.compile-pre.mk:114:
> > bnxt_ethdev.o] Error 1
> > gmake[5]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc/e6c3bc062b1045128114bdf0cfdd236b/dpdk/mk/rte.subdir.mk:37:
> > bnxt] Error 2
> > gmake[5]: *** Waiting for unfinished jobs....
>
>
> More than just that

OK, I see now... I'll spin a fix for this one:

> OS: FreeBSD12-64
> Target: x86_64-native-bsdapp-clang
> #define roundup(x, y)   ((((x)+((y)-1))/(y))*(y))  /* to any y */
>         ^
> 1 error generated.
> gmake[6]: *** [/tmp/FreeBSD12-64_K19.02_Clang6.0.1/x86_64-native-bsdapp-clang/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/internal/rte.compile-pre.mk:114: bnxt_ethdev.o] Error 1

Thanks,

   Lance

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [dpdk-dev] [PATCH v3 0/8] kni: fixes and cleanups
  2019-06-10 17:51 [dpdk-dev] [PATCH v2 0/7] kni: cleanups and improvements Stephen Hemminger
                   ` (7 preceding siblings ...)
  2019-06-11 20:54 ` [dpdk-dev] [PATCH v2 0/7] kni: cleanups and improvements Stephen Hemminger
@ 2019-06-18 16:16 ` Stephen Hemminger
  2019-06-18 16:16   ` [dpdk-dev] [PATCH v3 1/8] kni: don't need stubs for rx_mode or ioctl Stephen Hemminger
                     ` (7 more replies)
  2019-06-19 18:57 ` [dpdk-dev] [PATCH v3 0/8] kni: cleanups and fixes Stephen Hemminger
                   ` (3 subsequent siblings)
  12 siblings, 8 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-18 16:16 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

While testing KNI with netvsc, saw lots of places more code
could be safely removed from KNI kernel driver.

v3 - rebase to current master, add style fix patch
v2 - get rid of unnecessary padding, combine the unused field patches

Stephen Hemminger (8):
  kni: don't need stubs for rx_mode or ioctl
  kni: use netdev_alloc_skb
  kni: don't keep stats in kni_net
  kni: drop unused fields
  kni: use proper type for kni fifo's
  kni: return -EFAULT if copy_from_user fails
  doc: update KNI documentation
  kni: fix style issues

 .../sample_app_ug/kernel_nic_interface.rst    | 18 ++---
 kernel/linux/kni/kni_dev.h                    | 18 ++---
 kernel/linux/kni/kni_misc.c                   | 17 ++--
 kernel/linux/kni/kni_net.c                    | 79 +++++--------------
 lib/librte_kni/rte_kni.c                      | 30 +++----
 5 files changed, 53 insertions(+), 109 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 63+ messages in thread

* [dpdk-dev] [PATCH v3 1/8] kni: don't need stubs for rx_mode or ioctl
  2019-06-18 16:16 ` [dpdk-dev] [PATCH v3 0/8] kni: fixes and cleanups Stephen Hemminger
@ 2019-06-18 16:16   ` Stephen Hemminger
  2019-06-18 16:16   ` [dpdk-dev] [PATCH v3 2/8] kni: use netdev_alloc_skb Stephen Hemminger
                     ` (6 subsequent siblings)
  7 siblings, 0 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-18 16:16 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

The netdev subsystem already handles case where
network sevice does not support ioctl.

If device has no rx_mode hook it is not called.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 kernel/linux/kni/kni_net.c | 19 -------------------
 1 file changed, 19 deletions(-)

diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c
index ad8365877cda..c86337d099ab 100644
--- a/kernel/linux/kni/kni_net.c
+++ b/kernel/linux/kni/kni_net.c
@@ -593,23 +593,6 @@ kni_net_tx_timeout(struct net_device *dev)
 	netif_wake_queue(dev);
 }
 
-/*
- * Ioctl commands
- */
-static int
-kni_net_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
-{
-	pr_debug("kni_net_ioctl group:%d cmd:%d\n",
-		((struct kni_dev *)netdev_priv(dev))->group_id, cmd);
-
-	return -EOPNOTSUPP;
-}
-
-static void
-kni_net_set_rx_mode(struct net_device *dev)
-{
-}
-
 static int
 kni_net_change_mtu(struct net_device *dev, int new_mtu)
 {
@@ -758,8 +741,6 @@ static const struct net_device_ops kni_net_netdev_ops = {
 	.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,
-	.ndo_set_rx_mode = kni_net_set_rx_mode,
 	.ndo_get_stats = kni_net_stats,
 	.ndo_tx_timeout = kni_net_tx_timeout,
 	.ndo_set_mac_address = kni_net_set_mac,
-- 
2.20.1


^ permalink raw reply	[flat|nested] 63+ messages in thread

* [dpdk-dev] [PATCH v3 2/8] kni: use netdev_alloc_skb
  2019-06-18 16:16 ` [dpdk-dev] [PATCH v3 0/8] kni: fixes and cleanups Stephen Hemminger
  2019-06-18 16:16   ` [dpdk-dev] [PATCH v3 1/8] kni: don't need stubs for rx_mode or ioctl Stephen Hemminger
@ 2019-06-18 16:16   ` Stephen Hemminger
  2019-06-18 16:16   ` [dpdk-dev] [PATCH v3 3/8] kni: don't keep stats in kni_net Stephen Hemminger
                     ` (5 subsequent siblings)
  7 siblings, 0 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-18 16:16 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

netdev_alloc_skb is optimized to any alignment or setup
of skb->dev that is required. The kernel has chosen to not pad
packets on x86 (for many years), because it is faster.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 kernel/linux/kni/kni_net.c | 17 +++--------------
 1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c
index c86337d099ab..cce5e7eb991f 100644
--- a/kernel/linux/kni/kni_net.c
+++ b/kernel/linux/kni/kni_net.c
@@ -340,16 +340,13 @@ kni_net_rx_normal(struct kni_dev *kni)
 		data_kva = kva2data_kva(kva);
 		kni->va[i] = pa2va(kni->pa[i], kva);
 
-		skb = dev_alloc_skb(len + 2);
+		skb = netdev_alloc_skb(dev, len);
 		if (!skb) {
 			/* Update statistics */
 			kni->stats.rx_dropped++;
 			continue;
 		}
 
-		/* Align IP on 16B boundary */
-		skb_reserve(skb, 2);
-
 		if (kva->nb_segs == 1) {
 			memcpy(skb_put(skb, len), data_kva, len);
 		} else {
@@ -368,7 +365,6 @@ kni_net_rx_normal(struct kni_dev *kni)
 			}
 		}
 
-		skb->dev = dev;
 		skb->protocol = eth_type_trans(skb, dev);
 		skb->ip_summed = CHECKSUM_UNNECESSARY;
 
@@ -512,26 +508,20 @@ kni_net_rx_lo_fifo_skb(struct kni_dev *kni)
 		data_kva = kva2data_kva(kva);
 		kni->va[i] = pa2va(kni->pa[i], kva);
 
-		skb = dev_alloc_skb(len + 2);
+		skb = netdev_alloc_skb(dev, len);
 		if (skb) {
-			/* Align IP on 16B boundary */
-			skb_reserve(skb, 2);
 			memcpy(skb_put(skb, len), data_kva, len);
-			skb->dev = dev;
 			skb->ip_summed = CHECKSUM_UNNECESSARY;
 			dev_kfree_skb(skb);
 		}
 
 		/* Simulate real usage, allocate/copy skb twice */
-		skb = dev_alloc_skb(len + 2);
+		skb = netdev_alloc_skb(dev, len);
 		if (skb == NULL) {
 			kni->stats.rx_dropped++;
 			continue;
 		}
 
-		/* Align IP on 16B boundary */
-		skb_reserve(skb, 2);
-
 		if (kva->nb_segs == 1) {
 			memcpy(skb_put(skb, len), data_kva, len);
 		} else {
@@ -550,7 +540,6 @@ kni_net_rx_lo_fifo_skb(struct kni_dev *kni)
 			}
 		}
 
-		skb->dev = dev;
 		skb->ip_summed = CHECKSUM_UNNECESSARY;
 
 		kni->stats.rx_bytes += len;
-- 
2.20.1


^ permalink raw reply	[flat|nested] 63+ messages in thread

* [dpdk-dev] [PATCH v3 3/8] kni: don't keep stats in kni_net
  2019-06-18 16:16 ` [dpdk-dev] [PATCH v3 0/8] kni: fixes and cleanups Stephen Hemminger
  2019-06-18 16:16   ` [dpdk-dev] [PATCH v3 1/8] kni: don't need stubs for rx_mode or ioctl Stephen Hemminger
  2019-06-18 16:16   ` [dpdk-dev] [PATCH v3 2/8] kni: use netdev_alloc_skb Stephen Hemminger
@ 2019-06-18 16:16   ` Stephen Hemminger
  2019-06-18 16:16   ` [dpdk-dev] [PATCH v3 4/8] kni: drop unused fields Stephen Hemminger
                     ` (4 subsequent siblings)
  7 siblings, 0 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-18 16:16 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

Since kernel 2.6.28 the network subsystem has provided
dev->stats for devices to use statistics handling and is the
default if no ndo_get_stats is provided.

This allow allows for 64 bit (rather than just 32 bit)
statistics with KNI.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 kernel/linux/kni/kni_dev.h |  1 -
 kernel/linux/kni/kni_net.c | 43 +++++++++++++-------------------------
 2 files changed, 15 insertions(+), 29 deletions(-)

diff --git a/kernel/linux/kni/kni_dev.h b/kernel/linux/kni/kni_dev.h
index d57bce647e4a..21e4b0d92368 100644
--- a/kernel/linux/kni/kni_dev.h
+++ b/kernel/linux/kni/kni_dev.h
@@ -39,7 +39,6 @@ struct kni_dev {
 	/* kni list */
 	struct list_head list;
 
-	struct net_device_stats stats;
 	int status;
 	uint16_t group_id;           /* Group ID of a group of KNI devices */
 	uint32_t core_id;            /* Core ID to bind */
diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c
index cce5e7eb991f..06310fec57bb 100644
--- a/kernel/linux/kni/kni_net.c
+++ b/kernel/linux/kni/kni_net.c
@@ -291,15 +291,15 @@ kni_net_tx(struct sk_buff *skb, struct net_device *dev)
 
 	/* Free skb and update statistics */
 	dev_kfree_skb(skb);
-	kni->stats.tx_bytes += len;
-	kni->stats.tx_packets++;
+	dev->stats.tx_bytes += len;
+	dev->stats.tx_packets++;
 
 	return NETDEV_TX_OK;
 
 drop:
 	/* Free skb and update statistics */
 	dev_kfree_skb(skb);
-	kni->stats.tx_dropped++;
+	dev->stats.tx_dropped++;
 
 	return NETDEV_TX_OK;
 }
@@ -343,7 +343,7 @@ kni_net_rx_normal(struct kni_dev *kni)
 		skb = netdev_alloc_skb(dev, len);
 		if (!skb) {
 			/* Update statistics */
-			kni->stats.rx_dropped++;
+			dev->stats.rx_dropped++;
 			continue;
 		}
 
@@ -372,8 +372,8 @@ kni_net_rx_normal(struct kni_dev *kni)
 		netif_rx_ni(skb);
 
 		/* Update statistics */
-		kni->stats.rx_bytes += len;
-		kni->stats.rx_packets++;
+		dev->stats.rx_bytes += len;
+		dev->stats.rx_packets++;
 	}
 
 	/* Burst enqueue mbufs into free_q */
@@ -396,6 +396,7 @@ kni_net_rx_lo_fifo(struct kni_dev *kni)
 	void *data_kva;
 	struct rte_kni_mbuf *alloc_kva;
 	void *alloc_data_kva;
+	struct net_device *dev = kni->net_dev;
 
 	/* Get the number of entries in rx_q */
 	num_rq = kni_fifo_count(kni->rx_q);
@@ -443,8 +444,8 @@ kni_net_rx_lo_fifo(struct kni_dev *kni)
 			alloc_kva->pkt_len = len;
 			alloc_kva->data_len = len;
 
-			kni->stats.tx_bytes += len;
-			kni->stats.rx_bytes += len;
+			dev->stats.tx_bytes += len;
+			dev->stats.rx_bytes += len;
 		}
 
 		/* Burst enqueue mbufs into tx_q */
@@ -464,8 +465,8 @@ kni_net_rx_lo_fifo(struct kni_dev *kni)
 	 * Update statistic, and enqueue/dequeue failure is impossible,
 	 * as all queues are checked at first.
 	 */
-	kni->stats.tx_packets += num;
-	kni->stats.rx_packets += num;
+	dev->stats.tx_packets += num;
+	dev->stats.rx_packets += num;
 }
 
 /*
@@ -518,7 +519,7 @@ kni_net_rx_lo_fifo_skb(struct kni_dev *kni)
 		/* Simulate real usage, allocate/copy skb twice */
 		skb = netdev_alloc_skb(dev, len);
 		if (skb == NULL) {
-			kni->stats.rx_dropped++;
+			dev->stats.rx_dropped++;
 			continue;
 		}
 
@@ -542,8 +543,8 @@ kni_net_rx_lo_fifo_skb(struct kni_dev *kni)
 
 		skb->ip_summed = CHECKSUM_UNNECESSARY;
 
-		kni->stats.rx_bytes += len;
-		kni->stats.rx_packets++;
+		dev->stats.rx_bytes += len;
+		dev->stats.rx_packets++;
 
 		/* call tx interface */
 		kni_net_tx(skb, dev);
@@ -573,12 +574,10 @@ kni_net_rx(struct kni_dev *kni)
 static void
 kni_net_tx_timeout(struct net_device *dev)
 {
-	struct kni_dev *kni = netdev_priv(dev);
-
 	pr_debug("Transmit timeout at %ld, latency %ld\n", jiffies,
 			jiffies - dev_trans_start(dev));
 
-	kni->stats.tx_errors++;
+	dev->stats.tx_errors++;
 	netif_wake_queue(dev);
 }
 
@@ -627,17 +626,6 @@ kni_net_poll_resp(struct kni_dev *kni)
 		wake_up_interruptible(&kni->wq);
 }
 
-/*
- * Return statistics to the caller
- */
-static struct net_device_stats *
-kni_net_stats(struct net_device *dev)
-{
-	struct kni_dev *kni = netdev_priv(dev);
-
-	return &kni->stats;
-}
-
 /*
  *  Fill the eth header
  */
@@ -730,7 +718,6 @@ static const struct net_device_ops kni_net_netdev_ops = {
 	.ndo_change_rx_flags = kni_net_set_promiscusity,
 	.ndo_start_xmit = kni_net_tx,
 	.ndo_change_mtu = kni_net_change_mtu,
-	.ndo_get_stats = kni_net_stats,
 	.ndo_tx_timeout = kni_net_tx_timeout,
 	.ndo_set_mac_address = kni_net_set_mac,
 #ifdef HAVE_CHANGE_CARRIER_CB
-- 
2.20.1


^ permalink raw reply	[flat|nested] 63+ messages in thread

* [dpdk-dev] [PATCH v3 4/8] kni: drop unused fields
  2019-06-18 16:16 ` [dpdk-dev] [PATCH v3 0/8] kni: fixes and cleanups Stephen Hemminger
                     ` (2 preceding siblings ...)
  2019-06-18 16:16   ` [dpdk-dev] [PATCH v3 3/8] kni: don't keep stats in kni_net Stephen Hemminger
@ 2019-06-18 16:16   ` Stephen Hemminger
  2019-06-18 16:16   ` [dpdk-dev] [PATCH v3 5/8] kni: use proper type for kni fifo's Stephen Hemminger
                     ` (3 subsequent siblings)
  7 siblings, 0 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-18 16:16 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

The kni net structure only exists in driver no API/ABI.
Several fields were either totally unused or set and never used.

The fields in dev_info do need to stay in the ABI but
kernel can ignore them.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

kni: drop unused status element

Yet another ethtool leftover.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 kernel/linux/kni/kni_dev.h  | 5 -----
 kernel/linux/kni/kni_misc.c | 1 -
 2 files changed, 6 deletions(-)

diff --git a/kernel/linux/kni/kni_dev.h b/kernel/linux/kni/kni_dev.h
index 21e4b0d92368..f3e6c3ca4efa 100644
--- a/kernel/linux/kni/kni_dev.h
+++ b/kernel/linux/kni/kni_dev.h
@@ -39,8 +39,6 @@ struct kni_dev {
 	/* kni list */
 	struct list_head list;
 
-	int status;
-	uint16_t group_id;           /* Group ID of a group of KNI devices */
 	uint32_t core_id;            /* Core ID to bind */
 	char name[RTE_KNI_NAMESIZE]; /* Network device name */
 	struct task_struct *pthread;
@@ -79,9 +77,6 @@ struct kni_dev {
 	/* mbuf size */
 	uint32_t mbuf_size;
 
-	/* synchro for request processing */
-	unsigned long synchro;
-
 	/* buffers */
 	void *pa[MBUF_BURST_SZ];
 	void *va[MBUF_BURST_SZ];
diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c
index 1fc5eeb9c8ca..b59cf24c2184 100644
--- a/kernel/linux/kni/kni_misc.c
+++ b/kernel/linux/kni/kni_misc.c
@@ -346,7 +346,6 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num,
 	kni = netdev_priv(net_dev);
 
 	kni->net_dev = net_dev;
-	kni->group_id = dev_info.group_id;
 	kni->core_id = dev_info.core_id;
 	strncpy(kni->name, dev_info.name, RTE_KNI_NAMESIZE);
 
-- 
2.20.1


^ permalink raw reply	[flat|nested] 63+ messages in thread

* [dpdk-dev] [PATCH v3 5/8] kni: use proper type for kni fifo's
  2019-06-18 16:16 ` [dpdk-dev] [PATCH v3 0/8] kni: fixes and cleanups Stephen Hemminger
                     ` (3 preceding siblings ...)
  2019-06-18 16:16   ` [dpdk-dev] [PATCH v3 4/8] kni: drop unused fields Stephen Hemminger
@ 2019-06-18 16:16   ` Stephen Hemminger
  2019-06-18 16:16   ` [dpdk-dev] [PATCH v3 6/8] kni: return -EFAULT if copy_from_user fails Stephen Hemminger
                     ` (2 subsequent siblings)
  7 siblings, 0 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-18 16:16 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

Using void * instead of proper type is unsafe practice.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 kernel/linux/kni/kni_dev.h | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/kernel/linux/kni/kni_dev.h b/kernel/linux/kni/kni_dev.h
index f3e6c3ca4efa..ceba5f73c1d9 100644
--- a/kernel/linux/kni/kni_dev.h
+++ b/kernel/linux/kni/kni_dev.h
@@ -51,22 +51,22 @@ struct kni_dev {
 	struct net_device *net_dev;
 
 	/* queue for packets to be sent out */
-	void *tx_q;
+	struct rte_kni_fifo *tx_q;
 
 	/* queue for the packets received */
-	void *rx_q;
+	struct rte_kni_fifo *rx_q;
 
 	/* queue for the allocated mbufs those can be used to save sk buffs */
-	void *alloc_q;
+	struct rte_kni_fifo *alloc_q;
 
 	/* free queue for the mbufs to be freed */
-	void *free_q;
+	struct rte_kni_fifo *free_q;
 
 	/* request queue */
-	void *req_q;
+	struct rte_kni_fifo *req_q;
 
 	/* response queue */
-	void *resp_q;
+	struct rte_kni_fifo *resp_q;
 
 	void *sync_kva;
 	void *sync_va;
-- 
2.20.1


^ permalink raw reply	[flat|nested] 63+ messages in thread

* [dpdk-dev] [PATCH v3 6/8] kni: return -EFAULT if copy_from_user fails
  2019-06-18 16:16 ` [dpdk-dev] [PATCH v3 0/8] kni: fixes and cleanups Stephen Hemminger
                     ` (4 preceding siblings ...)
  2019-06-18 16:16   ` [dpdk-dev] [PATCH v3 5/8] kni: use proper type for kni fifo's Stephen Hemminger
@ 2019-06-18 16:16   ` Stephen Hemminger
  2019-06-18 16:16   ` [dpdk-dev] [PATCH v3 7/8] doc: update KNI documentation Stephen Hemminger
  2019-06-18 16:16   ` [dpdk-dev] [PATCH v3 8/8] kni: fix style issues Stephen Hemminger
  7 siblings, 0 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-18 16:16 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

The correct thing to return if user gives a bad data
is to return -EFAULT. Logging is also discouraged because
it could be used as a DoS attack.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 kernel/linux/kni/kni_misc.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c
index b59cf24c2184..be45f823408f 100644
--- a/kernel/linux/kni/kni_misc.c
+++ b/kernel/linux/kni/kni_misc.c
@@ -301,11 +301,8 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num,
 		return -EINVAL;
 
 	/* Copy kni info from user space */
-	ret = copy_from_user(&dev_info, (void *)ioctl_param, sizeof(dev_info));
-	if (ret) {
-		pr_err("copy_from_user in kni_ioctl_create");
-		return -EIO;
-	}
+	if (copy_from_user(&dev_info, (void *)ioctl_param, sizeof(dev_info)))
+		return -EFAULT;
 
 	/* Check if name is zero-ended */
 	if (strnlen(dev_info.name, sizeof(dev_info.name)) == sizeof(dev_info.name)) {
@@ -427,15 +424,12 @@ kni_ioctl_release(struct net *net, uint32_t ioctl_num,
 	if (_IOC_SIZE(ioctl_num) > sizeof(dev_info))
 		return -EINVAL;
 
-	ret = copy_from_user(&dev_info, (void *)ioctl_param, sizeof(dev_info));
-	if (ret) {
-		pr_err("copy_from_user in kni_ioctl_release");
-		return -EIO;
-	}
+	if (copy_from_user(&dev_info, (void *)ioctl_param, sizeof(dev_info)))
+		return -EFAULT;
 
 	/* Release the network device according to its name */
 	if (strlen(dev_info.name) == 0)
-		return ret;
+		return -EINVAL;
 
 	down_write(&knet->kni_list_lock);
 	list_for_each_entry_safe(dev, n, &knet->kni_list_head, list) {
-- 
2.20.1


^ permalink raw reply	[flat|nested] 63+ messages in thread

* [dpdk-dev] [PATCH v3 7/8] doc: update KNI documentation
  2019-06-18 16:16 ` [dpdk-dev] [PATCH v3 0/8] kni: fixes and cleanups Stephen Hemminger
                     ` (5 preceding siblings ...)
  2019-06-18 16:16   ` [dpdk-dev] [PATCH v3 6/8] kni: return -EFAULT if copy_from_user fails Stephen Hemminger
@ 2019-06-18 16:16   ` Stephen Hemminger
  2019-06-18 16:16   ` [dpdk-dev] [PATCH v3 8/8] kni: fix style issues Stephen Hemminger
  7 siblings, 0 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-18 16:16 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

Make the KNI documentation reflect modern kernel networking.
Ifconfig has been superseded by iproute2 for 15 years.
Iproute2 is well maintained, supports current feature set.

Ethtool is no longer supported by KNI.
Tshark is a better replacement for tcpdump.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 .../sample_app_ug/kernel_nic_interface.rst     | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/doc/guides/sample_app_ug/kernel_nic_interface.rst b/doc/guides/sample_app_ug/kernel_nic_interface.rst
index a7e549d5213a..422bd8c98465 100644
--- a/doc/guides/sample_app_ug/kernel_nic_interface.rst
+++ b/doc/guides/sample_app_ug/kernel_nic_interface.rst
@@ -21,14 +21,14 @@ The FIFO queues contain pointers to data packets in the DPDK. This:
 
 *   Provides a faster mechanism to interface with the kernel net stack and eliminates system calls
 
-*   Facilitates the DPDK using standard Linux* userspace net tools (tcpdump, ftp, and so on)
+*   Facilitates the DPDK using standard Linux* userspace net tools (tshark, rsync, and so on)
 
 *   Eliminate the copy_to_user and copy_from_user operations on packets.
 
 The Kernel NIC Interface sample application is a simple example that demonstrates the use
 of the DPDK to create a path for packets to go through the Linux* kernel.
 This is done by creating one or more kernel net devices for each of the DPDK ports.
-The application allows the use of standard Linux tools (ethtool, ifconfig, tcpdump) with the DPDK ports and
+The application allows the use of standard Linux tools (iproute, tshark) with the DPDK ports and
 also the exchange of packets between the DPDK application and the Linux* kernel.
 
 The Kernel NIC Interface sample application requires that the
@@ -220,13 +220,13 @@ Enable KNI interface and assign an IP address:
 
 .. code-block:: console
 
-    # ifconfig vEth0_0 192.168.0.1
+    # ip addr add dev vEth0_0 192.168.0.1
 
 Show KNI interface configuration and statistics:
 
 .. code-block:: console
 
-    # ifconfig vEth0_0
+    # ip -s -d addr show vEth0_0
 
 The user can also check and reset the packet statistics inside the ``kni``
 application by sending the app the USR1 and USR2 signals:
@@ -234,16 +234,16 @@ application by sending the app the USR1 and USR2 signals:
 .. code-block:: console
 
     # Print statistics
-    # kill -SIGUSR1 `pidof kni`
+    # pkill -USR1 kni
 
     # Zero statistics
-    # kill -SIGUSR2 `pidof kni`
+    # pkill -USR2 kni
 
 Dump network traffic:
 
 .. code-block:: console
 
-    # tcpdump -i vEth0_0
+    # tshark -n -i vEth0_0
 
 The normal Linux commands can also be used to change the MAC address and
 MTU size used by the physical NIC which corresponds to the KNI interface.
@@ -254,13 +254,13 @@ Change the MAC address:
 
 .. code-block:: console
 
-    # ifconfig vEth0_0 hw ether 0C:01:02:03:04:08
+    # ip link set dev vEth0_0 lladdr 0C:01:02:03:04:08
 
 Change the MTU size:
 
 .. code-block:: console
 
-    # ifconfig vEth0_0 mtu 1450
+    # ip link set dev vEth0_0 mtu 1450
 
 When the ``kni`` application is closed, all the KNI interfaces are deleted
 from the Linux kernel.
-- 
2.20.1


^ permalink raw reply	[flat|nested] 63+ messages in thread

* [dpdk-dev] [PATCH v3 8/8] kni: fix style issues
  2019-06-18 16:16 ` [dpdk-dev] [PATCH v3 0/8] kni: fixes and cleanups Stephen Hemminger
                     ` (6 preceding siblings ...)
  2019-06-18 16:16   ` [dpdk-dev] [PATCH v3 7/8] doc: update KNI documentation Stephen Hemminger
@ 2019-06-18 16:16   ` Stephen Hemminger
  7 siblings, 0 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-18 16:16 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

rte_kni does not follow standard style rules.
Noticed some extra \ line continuation etc.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_kni/rte_kni.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/lib/librte_kni/rte_kni.c b/lib/librte_kni/rte_kni.c
index e29d0cc7df3c..4828df3ca1e6 100644
--- a/lib/librte_kni/rte_kni.c
+++ b/lib/librte_kni/rte_kni.c
@@ -59,7 +59,7 @@ struct rte_kni {
 	uint16_t group_id;                  /**< Group ID of KNI devices */
 	uint32_t slot_id;                   /**< KNI pool slot ID */
 	struct rte_mempool *pktmbuf_pool;   /**< pkt mbuf mempool */
-	unsigned mbuf_size;                 /**< mbuf size */
+	unsigned int mbuf_size;                 /**< mbuf size */
 
 	const struct rte_memzone *m_tx_q;   /**< TX queue memzone */
 	const struct rte_memzone *m_rx_q;   /**< RX queue memzone */
@@ -78,7 +78,7 @@ struct rte_kni {
 	/* For request & response */
 	struct rte_kni_fifo *req_q;         /**< Request queue */
 	struct rte_kni_fifo *resp_q;        /**< Response queue */
-	void * sync_addr;                   /**< Req/Resp Mem address */
+	void *sync_addr;                   /**< Req/Resp Mem address */
 
 	struct rte_kni_ops ops;             /**< operations for request */
 };
@@ -473,7 +473,7 @@ kni_config_promiscusity(uint16_t port_id, uint8_t to_on)
 int
 rte_kni_handle_request(struct rte_kni *kni)
 {
-	unsigned ret;
+	unsigned int ret;
 	struct rte_kni_request *req = NULL;
 
 	if (kni == NULL)
@@ -498,8 +498,8 @@ rte_kni_handle_request(struct rte_kni *kni)
 		break;
 	case RTE_KNI_REQ_CFG_NETWORK_IF: /* Set network interface up/down */
 		if (kni->ops.config_network_if)
-			req->result = kni->ops.config_network_if(\
-					kni->ops.port_id, req->if_up);
+			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)
@@ -534,7 +534,7 @@ rte_kni_handle_request(struct rte_kni *kni)
 }
 
 unsigned
-rte_kni_tx_burst(struct rte_kni *kni, struct rte_mbuf **mbufs, unsigned num)
+rte_kni_tx_burst(struct rte_kni *kni, struct rte_mbuf **mbufs, unsigned int num)
 {
 	void *phy_mbufs[num];
 	unsigned int ret;
@@ -552,9 +552,9 @@ rte_kni_tx_burst(struct rte_kni *kni, struct rte_mbuf **mbufs, unsigned num)
 }
 
 unsigned
-rte_kni_rx_burst(struct rte_kni *kni, struct rte_mbuf **mbufs, unsigned num)
+rte_kni_rx_burst(struct rte_kni *kni, struct rte_mbuf **mbufs, unsigned int num)
 {
-	unsigned ret = kni_fifo_get(kni->tx_q, (void **)mbufs, num);
+	unsigned int ret = kni_fifo_get(kni->tx_q, (void **)mbufs, num);
 
 	/* If buffers removed, allocate mbufs and then put them into alloc_q */
 	if (ret)
@@ -605,7 +605,7 @@ kni_allocate_mbufs(struct rte_kni *kni)
 		return;
 	}
 
-	allocq_free = (kni->alloc_q->read - kni->alloc_q->write - 1) \
+	allocq_free = (kni->alloc_q->read - kni->alloc_q->write - 1)
 			& (MAX_MBUF_BURST_NUM - 1);
 	for (i = 0; i < allocq_free; i++) {
 		pkts[i] = rte_pktmbuf_alloc(kni->pktmbuf_pool);
@@ -659,7 +659,7 @@ static enum kni_ops_status
 kni_check_request_register(struct rte_kni_ops *ops)
 {
 	/* check if KNI request ops has been registered*/
-	if( NULL == ops )
+	if (NULL == ops)
 		return KNI_REQ_NO_REGISTER;
 
 	if ((ops->change_mtu == NULL)
@@ -672,22 +672,22 @@ kni_check_request_register(struct rte_kni_ops *ops)
 }
 
 int
-rte_kni_register_handlers(struct rte_kni *kni,struct rte_kni_ops *ops)
+rte_kni_register_handlers(struct rte_kni *kni, struct rte_kni_ops *ops)
 {
 	enum kni_ops_status req_status;
 
-	if (NULL == ops) {
+	if (ops == NULL) {
 		RTE_LOG(ERR, KNI, "Invalid KNI request operation.\n");
 		return -1;
 	}
 
-	if (NULL == kni) {
+	if (kni == NULL) {
 		RTE_LOG(ERR, KNI, "Invalid kni info.\n");
 		return -1;
 	}
 
 	req_status = kni_check_request_register(&kni->ops);
-	if ( KNI_REQ_REGISTERED == req_status) {
+	if (req_status == KNI_REQ_REGISTERED) {
 		RTE_LOG(ERR, KNI, "The KNI request operation has already registered.\n");
 		return -1;
 	}
@@ -699,7 +699,7 @@ rte_kni_register_handlers(struct rte_kni *kni,struct rte_kni_ops *ops)
 int
 rte_kni_unregister_handlers(struct rte_kni *kni)
 {
-	if (NULL == kni) {
+	if (kni == NULL) {
 		RTE_LOG(ERR, KNI, "Invalid kni info.\n");
 		return -1;
 	}
-- 
2.20.1


^ permalink raw reply	[flat|nested] 63+ messages in thread

* [dpdk-dev] [PATCH v3 0/8] kni: cleanups and fixes
  2019-06-10 17:51 [dpdk-dev] [PATCH v2 0/7] kni: cleanups and improvements Stephen Hemminger
                   ` (8 preceding siblings ...)
  2019-06-18 16:16 ` [dpdk-dev] [PATCH v3 0/8] kni: fixes and cleanups Stephen Hemminger
@ 2019-06-19 18:57 ` Stephen Hemminger
  2019-06-19 18:57   ` [dpdk-dev] [PATCH v3 1/8] kni: don't need stubs for rx_mode or ioctl Stephen Hemminger
                     ` (7 more replies)
  2019-06-19 18:59 ` [dpdk-dev] [PATCH v4 0/8] kni: fixes and cleanups Stephen Hemminger
                   ` (2 subsequent siblings)
  12 siblings, 8 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-19 18:57 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

While testing KNI with netvsc, saw lots of places more code
could be safely removed from KNI kernel driver.

This is still mostly "putting lipstick on a pig" all users
would be better off using virtio_user rather than KNI.

v3 - more style cleanups in last patch
v2 - get rid of unnecessary padding, combine the unused field patches

Stephen Hemminger (8):
  kni: don't need stubs for rx_mode or ioctl
  kni: use netdev_alloc_skb
  kni: don't keep stats in kni_net
  kni: drop unused fields
  kni: use proper type for kni fifo's
  kni: return -EFAULT if copy_from_user fails
  doc: update KNI documentation
  kni: fix style issues

 .../sample_app_ug/kernel_nic_interface.rst    | 18 ++---
 kernel/linux/kni/kni_dev.h                    | 18 ++---
 kernel/linux/kni/kni_misc.c                   | 17 ++--
 kernel/linux/kni/kni_net.c                    | 79 +++++--------------
 lib/librte_kni/rte_kni.c                      | 38 ++++-----
 5 files changed, 57 insertions(+), 113 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 63+ messages in thread

* [dpdk-dev] [PATCH v3 1/8] kni: don't need stubs for rx_mode or ioctl
  2019-06-19 18:57 ` [dpdk-dev] [PATCH v3 0/8] kni: cleanups and fixes Stephen Hemminger
@ 2019-06-19 18:57   ` Stephen Hemminger
  2019-06-19 18:57   ` [dpdk-dev] [PATCH v3 2/8] kni: use netdev_alloc_skb Stephen Hemminger
                     ` (6 subsequent siblings)
  7 siblings, 0 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-19 18:57 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, Stephen Hemminger

From: Stephen Hemminger <sthemmin@microsoft.com>

The netdev subsystem already handles case where
network sevice does not support ioctl.

If device has no rx_mode hook it is not called.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 kernel/linux/kni/kni_net.c | 19 -------------------
 1 file changed, 19 deletions(-)

diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c
index ad8365877cda..c86337d099ab 100644
--- a/kernel/linux/kni/kni_net.c
+++ b/kernel/linux/kni/kni_net.c
@@ -593,23 +593,6 @@ kni_net_tx_timeout(struct net_device *dev)
 	netif_wake_queue(dev);
 }
 
-/*
- * Ioctl commands
- */
-static int
-kni_net_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
-{
-	pr_debug("kni_net_ioctl group:%d cmd:%d\n",
-		((struct kni_dev *)netdev_priv(dev))->group_id, cmd);
-
-	return -EOPNOTSUPP;
-}
-
-static void
-kni_net_set_rx_mode(struct net_device *dev)
-{
-}
-
 static int
 kni_net_change_mtu(struct net_device *dev, int new_mtu)
 {
@@ -758,8 +741,6 @@ static const struct net_device_ops kni_net_netdev_ops = {
 	.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,
-	.ndo_set_rx_mode = kni_net_set_rx_mode,
 	.ndo_get_stats = kni_net_stats,
 	.ndo_tx_timeout = kni_net_tx_timeout,
 	.ndo_set_mac_address = kni_net_set_mac,
-- 
2.20.1


^ permalink raw reply	[flat|nested] 63+ messages in thread

* [dpdk-dev] [PATCH v3 2/8] kni: use netdev_alloc_skb
  2019-06-19 18:57 ` [dpdk-dev] [PATCH v3 0/8] kni: cleanups and fixes Stephen Hemminger
  2019-06-19 18:57   ` [dpdk-dev] [PATCH v3 1/8] kni: don't need stubs for rx_mode or ioctl Stephen Hemminger
@ 2019-06-19 18:57   ` Stephen Hemminger
  2019-06-19 18:57   ` [dpdk-dev] [PATCH v3 3/8] kni: don't keep stats in kni_net Stephen Hemminger
                     ` (5 subsequent siblings)
  7 siblings, 0 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-19 18:57 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, Stephen Hemminger

From: Stephen Hemminger <sthemmin@microsoft.com>

netdev_alloc_skb is optimized to any alignment or setup
of skb->dev that is required. The kernel has chosen to not pad
packets on x86 (for many years), because it is faster.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 kernel/linux/kni/kni_net.c | 17 +++--------------
 1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c
index c86337d099ab..cce5e7eb991f 100644
--- a/kernel/linux/kni/kni_net.c
+++ b/kernel/linux/kni/kni_net.c
@@ -340,16 +340,13 @@ kni_net_rx_normal(struct kni_dev *kni)
 		data_kva = kva2data_kva(kva);
 		kni->va[i] = pa2va(kni->pa[i], kva);
 
-		skb = dev_alloc_skb(len + 2);
+		skb = netdev_alloc_skb(dev, len);
 		if (!skb) {
 			/* Update statistics */
 			kni->stats.rx_dropped++;
 			continue;
 		}
 
-		/* Align IP on 16B boundary */
-		skb_reserve(skb, 2);
-
 		if (kva->nb_segs == 1) {
 			memcpy(skb_put(skb, len), data_kva, len);
 		} else {
@@ -368,7 +365,6 @@ kni_net_rx_normal(struct kni_dev *kni)
 			}
 		}
 
-		skb->dev = dev;
 		skb->protocol = eth_type_trans(skb, dev);
 		skb->ip_summed = CHECKSUM_UNNECESSARY;
 
@@ -512,26 +508,20 @@ kni_net_rx_lo_fifo_skb(struct kni_dev *kni)
 		data_kva = kva2data_kva(kva);
 		kni->va[i] = pa2va(kni->pa[i], kva);
 
-		skb = dev_alloc_skb(len + 2);
+		skb = netdev_alloc_skb(dev, len);
 		if (skb) {
-			/* Align IP on 16B boundary */
-			skb_reserve(skb, 2);
 			memcpy(skb_put(skb, len), data_kva, len);
-			skb->dev = dev;
 			skb->ip_summed = CHECKSUM_UNNECESSARY;
 			dev_kfree_skb(skb);
 		}
 
 		/* Simulate real usage, allocate/copy skb twice */
-		skb = dev_alloc_skb(len + 2);
+		skb = netdev_alloc_skb(dev, len);
 		if (skb == NULL) {
 			kni->stats.rx_dropped++;
 			continue;
 		}
 
-		/* Align IP on 16B boundary */
-		skb_reserve(skb, 2);
-
 		if (kva->nb_segs == 1) {
 			memcpy(skb_put(skb, len), data_kva, len);
 		} else {
@@ -550,7 +540,6 @@ kni_net_rx_lo_fifo_skb(struct kni_dev *kni)
 			}
 		}
 
-		skb->dev = dev;
 		skb->ip_summed = CHECKSUM_UNNECESSARY;
 
 		kni->stats.rx_bytes += len;
-- 
2.20.1


^ permalink raw reply	[flat|nested] 63+ messages in thread

* [dpdk-dev] [PATCH v3 3/8] kni: don't keep stats in kni_net
  2019-06-19 18:57 ` [dpdk-dev] [PATCH v3 0/8] kni: cleanups and fixes Stephen Hemminger
  2019-06-19 18:57   ` [dpdk-dev] [PATCH v3 1/8] kni: don't need stubs for rx_mode or ioctl Stephen Hemminger
  2019-06-19 18:57   ` [dpdk-dev] [PATCH v3 2/8] kni: use netdev_alloc_skb Stephen Hemminger
@ 2019-06-19 18:57   ` Stephen Hemminger
  2019-06-19 18:57   ` [dpdk-dev] [PATCH v3 4/8] kni: drop unused fields Stephen Hemminger
                     ` (4 subsequent siblings)
  7 siblings, 0 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-19 18:57 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, Stephen Hemminger

From: Stephen Hemminger <sthemmin@microsoft.com>

Since kernel 2.6.28 the network subsystem has provided
dev->stats for devices to use statistics handling and is the
default if no ndo_get_stats is provided.

This allow allows for 64 bit (rather than just 32 bit)
statistics with KNI.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 kernel/linux/kni/kni_dev.h |  1 -
 kernel/linux/kni/kni_net.c | 43 +++++++++++++-------------------------
 2 files changed, 15 insertions(+), 29 deletions(-)

diff --git a/kernel/linux/kni/kni_dev.h b/kernel/linux/kni/kni_dev.h
index d57bce647e4a..21e4b0d92368 100644
--- a/kernel/linux/kni/kni_dev.h
+++ b/kernel/linux/kni/kni_dev.h
@@ -39,7 +39,6 @@ struct kni_dev {
 	/* kni list */
 	struct list_head list;
 
-	struct net_device_stats stats;
 	int status;
 	uint16_t group_id;           /* Group ID of a group of KNI devices */
 	uint32_t core_id;            /* Core ID to bind */
diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c
index cce5e7eb991f..06310fec57bb 100644
--- a/kernel/linux/kni/kni_net.c
+++ b/kernel/linux/kni/kni_net.c
@@ -291,15 +291,15 @@ kni_net_tx(struct sk_buff *skb, struct net_device *dev)
 
 	/* Free skb and update statistics */
 	dev_kfree_skb(skb);
-	kni->stats.tx_bytes += len;
-	kni->stats.tx_packets++;
+	dev->stats.tx_bytes += len;
+	dev->stats.tx_packets++;
 
 	return NETDEV_TX_OK;
 
 drop:
 	/* Free skb and update statistics */
 	dev_kfree_skb(skb);
-	kni->stats.tx_dropped++;
+	dev->stats.tx_dropped++;
 
 	return NETDEV_TX_OK;
 }
@@ -343,7 +343,7 @@ kni_net_rx_normal(struct kni_dev *kni)
 		skb = netdev_alloc_skb(dev, len);
 		if (!skb) {
 			/* Update statistics */
-			kni->stats.rx_dropped++;
+			dev->stats.rx_dropped++;
 			continue;
 		}
 
@@ -372,8 +372,8 @@ kni_net_rx_normal(struct kni_dev *kni)
 		netif_rx_ni(skb);
 
 		/* Update statistics */
-		kni->stats.rx_bytes += len;
-		kni->stats.rx_packets++;
+		dev->stats.rx_bytes += len;
+		dev->stats.rx_packets++;
 	}
 
 	/* Burst enqueue mbufs into free_q */
@@ -396,6 +396,7 @@ kni_net_rx_lo_fifo(struct kni_dev *kni)
 	void *data_kva;
 	struct rte_kni_mbuf *alloc_kva;
 	void *alloc_data_kva;
+	struct net_device *dev = kni->net_dev;
 
 	/* Get the number of entries in rx_q */
 	num_rq = kni_fifo_count(kni->rx_q);
@@ -443,8 +444,8 @@ kni_net_rx_lo_fifo(struct kni_dev *kni)
 			alloc_kva->pkt_len = len;
 			alloc_kva->data_len = len;
 
-			kni->stats.tx_bytes += len;
-			kni->stats.rx_bytes += len;
+			dev->stats.tx_bytes += len;
+			dev->stats.rx_bytes += len;
 		}
 
 		/* Burst enqueue mbufs into tx_q */
@@ -464,8 +465,8 @@ kni_net_rx_lo_fifo(struct kni_dev *kni)
 	 * Update statistic, and enqueue/dequeue failure is impossible,
 	 * as all queues are checked at first.
 	 */
-	kni->stats.tx_packets += num;
-	kni->stats.rx_packets += num;
+	dev->stats.tx_packets += num;
+	dev->stats.rx_packets += num;
 }
 
 /*
@@ -518,7 +519,7 @@ kni_net_rx_lo_fifo_skb(struct kni_dev *kni)
 		/* Simulate real usage, allocate/copy skb twice */
 		skb = netdev_alloc_skb(dev, len);
 		if (skb == NULL) {
-			kni->stats.rx_dropped++;
+			dev->stats.rx_dropped++;
 			continue;
 		}
 
@@ -542,8 +543,8 @@ kni_net_rx_lo_fifo_skb(struct kni_dev *kni)
 
 		skb->ip_summed = CHECKSUM_UNNECESSARY;
 
-		kni->stats.rx_bytes += len;
-		kni->stats.rx_packets++;
+		dev->stats.rx_bytes += len;
+		dev->stats.rx_packets++;
 
 		/* call tx interface */
 		kni_net_tx(skb, dev);
@@ -573,12 +574,10 @@ kni_net_rx(struct kni_dev *kni)
 static void
 kni_net_tx_timeout(struct net_device *dev)
 {
-	struct kni_dev *kni = netdev_priv(dev);
-
 	pr_debug("Transmit timeout at %ld, latency %ld\n", jiffies,
 			jiffies - dev_trans_start(dev));
 
-	kni->stats.tx_errors++;
+	dev->stats.tx_errors++;
 	netif_wake_queue(dev);
 }
 
@@ -627,17 +626,6 @@ kni_net_poll_resp(struct kni_dev *kni)
 		wake_up_interruptible(&kni->wq);
 }
 
-/*
- * Return statistics to the caller
- */
-static struct net_device_stats *
-kni_net_stats(struct net_device *dev)
-{
-	struct kni_dev *kni = netdev_priv(dev);
-
-	return &kni->stats;
-}
-
 /*
  *  Fill the eth header
  */
@@ -730,7 +718,6 @@ static const struct net_device_ops kni_net_netdev_ops = {
 	.ndo_change_rx_flags = kni_net_set_promiscusity,
 	.ndo_start_xmit = kni_net_tx,
 	.ndo_change_mtu = kni_net_change_mtu,
-	.ndo_get_stats = kni_net_stats,
 	.ndo_tx_timeout = kni_net_tx_timeout,
 	.ndo_set_mac_address = kni_net_set_mac,
 #ifdef HAVE_CHANGE_CARRIER_CB
-- 
2.20.1


^ permalink raw reply	[flat|nested] 63+ messages in thread

* [dpdk-dev] [PATCH v3 4/8] kni: drop unused fields
  2019-06-19 18:57 ` [dpdk-dev] [PATCH v3 0/8] kni: cleanups and fixes Stephen Hemminger
                     ` (2 preceding siblings ...)
  2019-06-19 18:57   ` [dpdk-dev] [PATCH v3 3/8] kni: don't keep stats in kni_net Stephen Hemminger
@ 2019-06-19 18:57   ` Stephen Hemminger
  2019-06-19 18:57   ` [dpdk-dev] [PATCH v3 5/8] kni: use proper type for kni fifo's Stephen Hemminger
                     ` (3 subsequent siblings)
  7 siblings, 0 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-19 18:57 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, Stephen Hemminger

From: Stephen Hemminger <sthemmin@microsoft.com>

The kni net structure only exists in driver no API/ABI.
Several fields were either totally unused or set and never used.

The fields in dev_info do need to stay in the ABI but
kernel can ignore them.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

kni: drop unused status element

Yet another ethtool leftover.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 kernel/linux/kni/kni_dev.h  | 5 -----
 kernel/linux/kni/kni_misc.c | 1 -
 2 files changed, 6 deletions(-)

diff --git a/kernel/linux/kni/kni_dev.h b/kernel/linux/kni/kni_dev.h
index 21e4b0d92368..f3e6c3ca4efa 100644
--- a/kernel/linux/kni/kni_dev.h
+++ b/kernel/linux/kni/kni_dev.h
@@ -39,8 +39,6 @@ struct kni_dev {
 	/* kni list */
 	struct list_head list;
 
-	int status;
-	uint16_t group_id;           /* Group ID of a group of KNI devices */
 	uint32_t core_id;            /* Core ID to bind */
 	char name[RTE_KNI_NAMESIZE]; /* Network device name */
 	struct task_struct *pthread;
@@ -79,9 +77,6 @@ struct kni_dev {
 	/* mbuf size */
 	uint32_t mbuf_size;
 
-	/* synchro for request processing */
-	unsigned long synchro;
-
 	/* buffers */
 	void *pa[MBUF_BURST_SZ];
 	void *va[MBUF_BURST_SZ];
diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c
index 1fc5eeb9c8ca..b59cf24c2184 100644
--- a/kernel/linux/kni/kni_misc.c
+++ b/kernel/linux/kni/kni_misc.c
@@ -346,7 +346,6 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num,
 	kni = netdev_priv(net_dev);
 
 	kni->net_dev = net_dev;
-	kni->group_id = dev_info.group_id;
 	kni->core_id = dev_info.core_id;
 	strncpy(kni->name, dev_info.name, RTE_KNI_NAMESIZE);
 
-- 
2.20.1


^ permalink raw reply	[flat|nested] 63+ messages in thread

* [dpdk-dev] [PATCH v3 5/8] kni: use proper type for kni fifo's
  2019-06-19 18:57 ` [dpdk-dev] [PATCH v3 0/8] kni: cleanups and fixes Stephen Hemminger
                     ` (3 preceding siblings ...)
  2019-06-19 18:57   ` [dpdk-dev] [PATCH v3 4/8] kni: drop unused fields Stephen Hemminger
@ 2019-06-19 18:57   ` Stephen Hemminger
  2019-06-19 18:57   ` [dpdk-dev] [PATCH v3 6/8] kni: return -EFAULT if copy_from_user fails Stephen Hemminger
                     ` (2 subsequent siblings)
  7 siblings, 0 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-19 18:57 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, Stephen Hemminger

From: Stephen Hemminger <sthemmin@microsoft.com>

Using void * instead of proper type is unsafe practice.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 kernel/linux/kni/kni_dev.h | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/kernel/linux/kni/kni_dev.h b/kernel/linux/kni/kni_dev.h
index f3e6c3ca4efa..ceba5f73c1d9 100644
--- a/kernel/linux/kni/kni_dev.h
+++ b/kernel/linux/kni/kni_dev.h
@@ -51,22 +51,22 @@ struct kni_dev {
 	struct net_device *net_dev;
 
 	/* queue for packets to be sent out */
-	void *tx_q;
+	struct rte_kni_fifo *tx_q;
 
 	/* queue for the packets received */
-	void *rx_q;
+	struct rte_kni_fifo *rx_q;
 
 	/* queue for the allocated mbufs those can be used to save sk buffs */
-	void *alloc_q;
+	struct rte_kni_fifo *alloc_q;
 
 	/* free queue for the mbufs to be freed */
-	void *free_q;
+	struct rte_kni_fifo *free_q;
 
 	/* request queue */
-	void *req_q;
+	struct rte_kni_fifo *req_q;
 
 	/* response queue */
-	void *resp_q;
+	struct rte_kni_fifo *resp_q;
 
 	void *sync_kva;
 	void *sync_va;
-- 
2.20.1


^ permalink raw reply	[flat|nested] 63+ messages in thread

* [dpdk-dev] [PATCH v3 6/8] kni: return -EFAULT if copy_from_user fails
  2019-06-19 18:57 ` [dpdk-dev] [PATCH v3 0/8] kni: cleanups and fixes Stephen Hemminger
                     ` (4 preceding siblings ...)
  2019-06-19 18:57   ` [dpdk-dev] [PATCH v3 5/8] kni: use proper type for kni fifo's Stephen Hemminger
@ 2019-06-19 18:57   ` Stephen Hemminger
  2019-06-19 18:57   ` [dpdk-dev] [PATCH v3 7/8] doc: update KNI documentation Stephen Hemminger
  2019-06-19 18:57   ` [dpdk-dev] [PATCH v3 8/8] kni: fix style issues Stephen Hemminger
  7 siblings, 0 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-19 18:57 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, Stephen Hemminger

From: Stephen Hemminger <sthemmin@microsoft.com>

The correct thing to return if user gives a bad data
is to return -EFAULT. Logging is also discouraged because
it could be used as a DoS attack.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 kernel/linux/kni/kni_misc.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c
index b59cf24c2184..be45f823408f 100644
--- a/kernel/linux/kni/kni_misc.c
+++ b/kernel/linux/kni/kni_misc.c
@@ -301,11 +301,8 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num,
 		return -EINVAL;
 
 	/* Copy kni info from user space */
-	ret = copy_from_user(&dev_info, (void *)ioctl_param, sizeof(dev_info));
-	if (ret) {
-		pr_err("copy_from_user in kni_ioctl_create");
-		return -EIO;
-	}
+	if (copy_from_user(&dev_info, (void *)ioctl_param, sizeof(dev_info)))
+		return -EFAULT;
 
 	/* Check if name is zero-ended */
 	if (strnlen(dev_info.name, sizeof(dev_info.name)) == sizeof(dev_info.name)) {
@@ -427,15 +424,12 @@ kni_ioctl_release(struct net *net, uint32_t ioctl_num,
 	if (_IOC_SIZE(ioctl_num) > sizeof(dev_info))
 		return -EINVAL;
 
-	ret = copy_from_user(&dev_info, (void *)ioctl_param, sizeof(dev_info));
-	if (ret) {
-		pr_err("copy_from_user in kni_ioctl_release");
-		return -EIO;
-	}
+	if (copy_from_user(&dev_info, (void *)ioctl_param, sizeof(dev_info)))
+		return -EFAULT;
 
 	/* Release the network device according to its name */
 	if (strlen(dev_info.name) == 0)
-		return ret;
+		return -EINVAL;
 
 	down_write(&knet->kni_list_lock);
 	list_for_each_entry_safe(dev, n, &knet->kni_list_head, list) {
-- 
2.20.1


^ permalink raw reply	[flat|nested] 63+ messages in thread

* [dpdk-dev] [PATCH v3 7/8] doc: update KNI documentation
  2019-06-19 18:57 ` [dpdk-dev] [PATCH v3 0/8] kni: cleanups and fixes Stephen Hemminger
                     ` (5 preceding siblings ...)
  2019-06-19 18:57   ` [dpdk-dev] [PATCH v3 6/8] kni: return -EFAULT if copy_from_user fails Stephen Hemminger
@ 2019-06-19 18:57   ` Stephen Hemminger
  2019-06-19 18:57   ` [dpdk-dev] [PATCH v3 8/8] kni: fix style issues Stephen Hemminger
  7 siblings, 0 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-19 18:57 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, Stephen Hemminger

From: Stephen Hemminger <sthemmin@microsoft.com>

Make the KNI documentation reflect modern kernel networking.
Ifconfig has been superseded by iproute2 for 15 years.
Iproute2 is well maintained, supports current feature set.

Ethtool is no longer supported by KNI.
Tshark is a better replacement for tcpdump.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 .../sample_app_ug/kernel_nic_interface.rst     | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/doc/guides/sample_app_ug/kernel_nic_interface.rst b/doc/guides/sample_app_ug/kernel_nic_interface.rst
index a7e549d5213a..422bd8c98465 100644
--- a/doc/guides/sample_app_ug/kernel_nic_interface.rst
+++ b/doc/guides/sample_app_ug/kernel_nic_interface.rst
@@ -21,14 +21,14 @@ The FIFO queues contain pointers to data packets in the DPDK. This:
 
 *   Provides a faster mechanism to interface with the kernel net stack and eliminates system calls
 
-*   Facilitates the DPDK using standard Linux* userspace net tools (tcpdump, ftp, and so on)
+*   Facilitates the DPDK using standard Linux* userspace net tools (tshark, rsync, and so on)
 
 *   Eliminate the copy_to_user and copy_from_user operations on packets.
 
 The Kernel NIC Interface sample application is a simple example that demonstrates the use
 of the DPDK to create a path for packets to go through the Linux* kernel.
 This is done by creating one or more kernel net devices for each of the DPDK ports.
-The application allows the use of standard Linux tools (ethtool, ifconfig, tcpdump) with the DPDK ports and
+The application allows the use of standard Linux tools (iproute, tshark) with the DPDK ports and
 also the exchange of packets between the DPDK application and the Linux* kernel.
 
 The Kernel NIC Interface sample application requires that the
@@ -220,13 +220,13 @@ Enable KNI interface and assign an IP address:
 
 .. code-block:: console
 
-    # ifconfig vEth0_0 192.168.0.1
+    # ip addr add dev vEth0_0 192.168.0.1
 
 Show KNI interface configuration and statistics:
 
 .. code-block:: console
 
-    # ifconfig vEth0_0
+    # ip -s -d addr show vEth0_0
 
 The user can also check and reset the packet statistics inside the ``kni``
 application by sending the app the USR1 and USR2 signals:
@@ -234,16 +234,16 @@ application by sending the app the USR1 and USR2 signals:
 .. code-block:: console
 
     # Print statistics
-    # kill -SIGUSR1 `pidof kni`
+    # pkill -USR1 kni
 
     # Zero statistics
-    # kill -SIGUSR2 `pidof kni`
+    # pkill -USR2 kni
 
 Dump network traffic:
 
 .. code-block:: console
 
-    # tcpdump -i vEth0_0
+    # tshark -n -i vEth0_0
 
 The normal Linux commands can also be used to change the MAC address and
 MTU size used by the physical NIC which corresponds to the KNI interface.
@@ -254,13 +254,13 @@ Change the MAC address:
 
 .. code-block:: console
 
-    # ifconfig vEth0_0 hw ether 0C:01:02:03:04:08
+    # ip link set dev vEth0_0 lladdr 0C:01:02:03:04:08
 
 Change the MTU size:
 
 .. code-block:: console
 
-    # ifconfig vEth0_0 mtu 1450
+    # ip link set dev vEth0_0 mtu 1450
 
 When the ``kni`` application is closed, all the KNI interfaces are deleted
 from the Linux kernel.
-- 
2.20.1


^ permalink raw reply	[flat|nested] 63+ messages in thread

* [dpdk-dev] [PATCH v3 8/8] kni: fix style issues
  2019-06-19 18:57 ` [dpdk-dev] [PATCH v3 0/8] kni: cleanups and fixes Stephen Hemminger
                     ` (6 preceding siblings ...)
  2019-06-19 18:57   ` [dpdk-dev] [PATCH v3 7/8] doc: update KNI documentation Stephen Hemminger
@ 2019-06-19 18:57   ` Stephen Hemminger
  7 siblings, 0 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-19 18:57 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, Stephen Hemminger

From: Stephen Hemminger <sthemmin@microsoft.com>

rte_kni does not follow standard style rules.
Noticed some extra \ line continuation etc.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_kni/rte_kni.c | 38 +++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/lib/librte_kni/rte_kni.c b/lib/librte_kni/rte_kni.c
index e29d0cc7df3c..9b6acc382fc3 100644
--- a/lib/librte_kni/rte_kni.c
+++ b/lib/librte_kni/rte_kni.c
@@ -59,7 +59,7 @@ struct rte_kni {
 	uint16_t group_id;                  /**< Group ID of KNI devices */
 	uint32_t slot_id;                   /**< KNI pool slot ID */
 	struct rte_mempool *pktmbuf_pool;   /**< pkt mbuf mempool */
-	unsigned mbuf_size;                 /**< mbuf size */
+	unsigned int mbuf_size;                 /**< mbuf size */
 
 	const struct rte_memzone *m_tx_q;   /**< TX queue memzone */
 	const struct rte_memzone *m_rx_q;   /**< RX queue memzone */
@@ -78,7 +78,7 @@ struct rte_kni {
 	/* For request & response */
 	struct rte_kni_fifo *req_q;         /**< Request queue */
 	struct rte_kni_fifo *resp_q;        /**< Response queue */
-	void * sync_addr;                   /**< Req/Resp Mem address */
+	void *sync_addr;                   /**< Req/Resp Mem address */
 
 	struct rte_kni_ops ops;             /**< operations for request */
 };
@@ -473,7 +473,7 @@ kni_config_promiscusity(uint16_t port_id, uint8_t to_on)
 int
 rte_kni_handle_request(struct rte_kni *kni)
 {
-	unsigned ret;
+	unsigned int ret;
 	struct rte_kni_request *req = NULL;
 
 	if (kni == NULL)
@@ -498,8 +498,8 @@ rte_kni_handle_request(struct rte_kni *kni)
 		break;
 	case RTE_KNI_REQ_CFG_NETWORK_IF: /* Set network interface up/down */
 		if (kni->ops.config_network_if)
-			req->result = kni->ops.config_network_if(\
-					kni->ops.port_id, req->if_up);
+			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)
@@ -534,7 +534,7 @@ rte_kni_handle_request(struct rte_kni *kni)
 }
 
 unsigned
-rte_kni_tx_burst(struct rte_kni *kni, struct rte_mbuf **mbufs, unsigned num)
+rte_kni_tx_burst(struct rte_kni *kni, struct rte_mbuf **mbufs, unsigned int num)
 {
 	void *phy_mbufs[num];
 	unsigned int ret;
@@ -552,9 +552,9 @@ rte_kni_tx_burst(struct rte_kni *kni, struct rte_mbuf **mbufs, unsigned num)
 }
 
 unsigned
-rte_kni_rx_burst(struct rte_kni *kni, struct rte_mbuf **mbufs, unsigned num)
+rte_kni_rx_burst(struct rte_kni *kni, struct rte_mbuf **mbufs, unsigned int num)
 {
-	unsigned ret = kni_fifo_get(kni->tx_q, (void **)mbufs, num);
+	unsigned int ret = kni_fifo_get(kni->tx_q, (void **)mbufs, num);
 
 	/* If buffers removed, allocate mbufs and then put them into alloc_q */
 	if (ret)
@@ -605,7 +605,7 @@ kni_allocate_mbufs(struct rte_kni *kni)
 		return;
 	}
 
-	allocq_free = (kni->alloc_q->read - kni->alloc_q->write - 1) \
+	allocq_free = (kni->alloc_q->read - kni->alloc_q->write - 1)
 			& (MAX_MBUF_BURST_NUM - 1);
 	for (i = 0; i < allocq_free; i++) {
 		pkts[i] = rte_pktmbuf_alloc(kni->pktmbuf_pool);
@@ -659,35 +659,35 @@ static enum kni_ops_status
 kni_check_request_register(struct rte_kni_ops *ops)
 {
 	/* check if KNI request ops has been registered*/
-	if( NULL == ops )
+	if (ops == NULL)
 		return KNI_REQ_NO_REGISTER;
 
-	if ((ops->change_mtu == NULL)
-		&& (ops->config_network_if == NULL)
-		&& (ops->config_mac_address == NULL)
-		&& (ops->config_promiscusity == NULL))
+	if (ops->change_mtu == NULL
+	    && ops->config_network_if == NULL
+	    && ops->config_mac_address == NULL
+	    && ops->config_promiscusity == NULL)
 		return KNI_REQ_NO_REGISTER;
 
 	return KNI_REQ_REGISTERED;
 }
 
 int
-rte_kni_register_handlers(struct rte_kni *kni,struct rte_kni_ops *ops)
+rte_kni_register_handlers(struct rte_kni *kni, struct rte_kni_ops *ops)
 {
 	enum kni_ops_status req_status;
 
-	if (NULL == ops) {
+	if (ops == NULL) {
 		RTE_LOG(ERR, KNI, "Invalid KNI request operation.\n");
 		return -1;
 	}
 
-	if (NULL == kni) {
+	if (kni == NULL) {
 		RTE_LOG(ERR, KNI, "Invalid kni info.\n");
 		return -1;
 	}
 
 	req_status = kni_check_request_register(&kni->ops);
-	if ( KNI_REQ_REGISTERED == req_status) {
+	if (req_status == KNI_REQ_REGISTERED) {
 		RTE_LOG(ERR, KNI, "The KNI request operation has already registered.\n");
 		return -1;
 	}
@@ -699,7 +699,7 @@ rte_kni_register_handlers(struct rte_kni *kni,struct rte_kni_ops *ops)
 int
 rte_kni_unregister_handlers(struct rte_kni *kni)
 {
-	if (NULL == kni) {
+	if (kni == NULL) {
 		RTE_LOG(ERR, KNI, "Invalid kni info.\n");
 		return -1;
 	}
-- 
2.20.1


^ permalink raw reply	[flat|nested] 63+ messages in thread

* [dpdk-dev] [PATCH v4 0/8] kni: fixes and cleanups
  2019-06-10 17:51 [dpdk-dev] [PATCH v2 0/7] kni: cleanups and improvements Stephen Hemminger
                   ` (9 preceding siblings ...)
  2019-06-19 18:57 ` [dpdk-dev] [PATCH v3 0/8] kni: cleanups and fixes Stephen Hemminger
@ 2019-06-19 18:59 ` Stephen Hemminger
  2019-06-19 18:59   ` [dpdk-dev] [PATCH v4 1/8] kni: don't need stubs for rx_mode or ioctl Stephen Hemminger
                     ` (7 more replies)
  2019-06-20 19:20 ` [dpdk-dev] [PATCH v5 0/9] kni: fixes and cleanups Stephen Hemminger
  2019-06-24 16:47 ` [dpdk-dev] [PATCH v6 0/9] kni: fixes and cleanups Stephen Hemminger
  12 siblings, 8 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-19 18:59 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

While testing KNI with netvsc, saw lots of places more code
could be safely removed from KNI kernel driver.

v4 - add more style fixes
v3 - rebase to current master, add style fix patch
v2 - get rid of unnecessary padding, combine the unused field patches

Stephen Hemminger (8):
  kni: don't need stubs for rx_mode or ioctl
  kni: use netdev_alloc_skb
  kni: don't keep stats in kni_net
  kni: drop unused fields
  kni: use proper type for kni fifo's
  kni: return -EFAULT if copy_from_user fails
  doc: update KNI documentation
  kni: fix style issues

 .../sample_app_ug/kernel_nic_interface.rst    | 18 ++---
 kernel/linux/kni/kni_dev.h                    | 18 ++---
 kernel/linux/kni/kni_misc.c                   | 17 ++--
 kernel/linux/kni/kni_net.c                    | 79 +++++--------------
 lib/librte_kni/rte_kni.c                      | 38 ++++-----
 5 files changed, 57 insertions(+), 113 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 63+ messages in thread

* [dpdk-dev] [PATCH v4 1/8] kni: don't need stubs for rx_mode or ioctl
  2019-06-19 18:59 ` [dpdk-dev] [PATCH v4 0/8] kni: fixes and cleanups Stephen Hemminger
@ 2019-06-19 18:59   ` Stephen Hemminger
  2019-06-19 18:59   ` [dpdk-dev] [PATCH v4 2/8] kni: use netdev_alloc_skb Stephen Hemminger
                     ` (6 subsequent siblings)
  7 siblings, 0 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-19 18:59 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, Stephen Hemminger

From: Stephen Hemminger <sthemmin@microsoft.com>

The netdev subsystem already handles case where
network sevice does not support ioctl.

If device has no rx_mode hook it is not called.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 kernel/linux/kni/kni_net.c | 19 -------------------
 1 file changed, 19 deletions(-)

diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c
index ad8365877cda..c86337d099ab 100644
--- a/kernel/linux/kni/kni_net.c
+++ b/kernel/linux/kni/kni_net.c
@@ -593,23 +593,6 @@ kni_net_tx_timeout(struct net_device *dev)
 	netif_wake_queue(dev);
 }
 
-/*
- * Ioctl commands
- */
-static int
-kni_net_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
-{
-	pr_debug("kni_net_ioctl group:%d cmd:%d\n",
-		((struct kni_dev *)netdev_priv(dev))->group_id, cmd);
-
-	return -EOPNOTSUPP;
-}
-
-static void
-kni_net_set_rx_mode(struct net_device *dev)
-{
-}
-
 static int
 kni_net_change_mtu(struct net_device *dev, int new_mtu)
 {
@@ -758,8 +741,6 @@ static const struct net_device_ops kni_net_netdev_ops = {
 	.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,
-	.ndo_set_rx_mode = kni_net_set_rx_mode,
 	.ndo_get_stats = kni_net_stats,
 	.ndo_tx_timeout = kni_net_tx_timeout,
 	.ndo_set_mac_address = kni_net_set_mac,
-- 
2.20.1


^ permalink raw reply	[flat|nested] 63+ messages in thread

* [dpdk-dev] [PATCH v4 2/8] kni: use netdev_alloc_skb
  2019-06-19 18:59 ` [dpdk-dev] [PATCH v4 0/8] kni: fixes and cleanups Stephen Hemminger
  2019-06-19 18:59   ` [dpdk-dev] [PATCH v4 1/8] kni: don't need stubs for rx_mode or ioctl Stephen Hemminger
@ 2019-06-19 18:59   ` Stephen Hemminger
  2019-06-19 18:59   ` [dpdk-dev] [PATCH v4 3/8] kni: don't keep stats in kni_net Stephen Hemminger
                     ` (5 subsequent siblings)
  7 siblings, 0 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-19 18:59 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, Stephen Hemminger

From: Stephen Hemminger <sthemmin@microsoft.com>

netdev_alloc_skb is optimized to any alignment or setup
of skb->dev that is required. The kernel has chosen to not pad
packets on x86 (for many years), because it is faster.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 kernel/linux/kni/kni_net.c | 17 +++--------------
 1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c
index c86337d099ab..cce5e7eb991f 100644
--- a/kernel/linux/kni/kni_net.c
+++ b/kernel/linux/kni/kni_net.c
@@ -340,16 +340,13 @@ kni_net_rx_normal(struct kni_dev *kni)
 		data_kva = kva2data_kva(kva);
 		kni->va[i] = pa2va(kni->pa[i], kva);
 
-		skb = dev_alloc_skb(len + 2);
+		skb = netdev_alloc_skb(dev, len);
 		if (!skb) {
 			/* Update statistics */
 			kni->stats.rx_dropped++;
 			continue;
 		}
 
-		/* Align IP on 16B boundary */
-		skb_reserve(skb, 2);
-
 		if (kva->nb_segs == 1) {
 			memcpy(skb_put(skb, len), data_kva, len);
 		} else {
@@ -368,7 +365,6 @@ kni_net_rx_normal(struct kni_dev *kni)
 			}
 		}
 
-		skb->dev = dev;
 		skb->protocol = eth_type_trans(skb, dev);
 		skb->ip_summed = CHECKSUM_UNNECESSARY;
 
@@ -512,26 +508,20 @@ kni_net_rx_lo_fifo_skb(struct kni_dev *kni)
 		data_kva = kva2data_kva(kva);
 		kni->va[i] = pa2va(kni->pa[i], kva);
 
-		skb = dev_alloc_skb(len + 2);
+		skb = netdev_alloc_skb(dev, len);
 		if (skb) {
-			/* Align IP on 16B boundary */
-			skb_reserve(skb, 2);
 			memcpy(skb_put(skb, len), data_kva, len);
-			skb->dev = dev;
 			skb->ip_summed = CHECKSUM_UNNECESSARY;
 			dev_kfree_skb(skb);
 		}
 
 		/* Simulate real usage, allocate/copy skb twice */
-		skb = dev_alloc_skb(len + 2);
+		skb = netdev_alloc_skb(dev, len);
 		if (skb == NULL) {
 			kni->stats.rx_dropped++;
 			continue;
 		}
 
-		/* Align IP on 16B boundary */
-		skb_reserve(skb, 2);
-
 		if (kva->nb_segs == 1) {
 			memcpy(skb_put(skb, len), data_kva, len);
 		} else {
@@ -550,7 +540,6 @@ kni_net_rx_lo_fifo_skb(struct kni_dev *kni)
 			}
 		}
 
-		skb->dev = dev;
 		skb->ip_summed = CHECKSUM_UNNECESSARY;
 
 		kni->stats.rx_bytes += len;
-- 
2.20.1


^ permalink raw reply	[flat|nested] 63+ messages in thread

* [dpdk-dev] [PATCH v4 3/8] kni: don't keep stats in kni_net
  2019-06-19 18:59 ` [dpdk-dev] [PATCH v4 0/8] kni: fixes and cleanups Stephen Hemminger
  2019-06-19 18:59   ` [dpdk-dev] [PATCH v4 1/8] kni: don't need stubs for rx_mode or ioctl Stephen Hemminger
  2019-06-19 18:59   ` [dpdk-dev] [PATCH v4 2/8] kni: use netdev_alloc_skb Stephen Hemminger
@ 2019-06-19 18:59   ` Stephen Hemminger
  2019-06-19 18:59   ` [dpdk-dev] [PATCH v4 4/8] kni: drop unused fields Stephen Hemminger
                     ` (4 subsequent siblings)
  7 siblings, 0 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-19 18:59 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, Stephen Hemminger

From: Stephen Hemminger <sthemmin@microsoft.com>

Since kernel 2.6.28 the network subsystem has provided
dev->stats for devices to use statistics handling and is the
default if no ndo_get_stats is provided.

This allow allows for 64 bit (rather than just 32 bit)
statistics with KNI.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 kernel/linux/kni/kni_dev.h |  1 -
 kernel/linux/kni/kni_net.c | 43 +++++++++++++-------------------------
 2 files changed, 15 insertions(+), 29 deletions(-)

diff --git a/kernel/linux/kni/kni_dev.h b/kernel/linux/kni/kni_dev.h
index d57bce647e4a..21e4b0d92368 100644
--- a/kernel/linux/kni/kni_dev.h
+++ b/kernel/linux/kni/kni_dev.h
@@ -39,7 +39,6 @@ struct kni_dev {
 	/* kni list */
 	struct list_head list;
 
-	struct net_device_stats stats;
 	int status;
 	uint16_t group_id;           /* Group ID of a group of KNI devices */
 	uint32_t core_id;            /* Core ID to bind */
diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c
index cce5e7eb991f..06310fec57bb 100644
--- a/kernel/linux/kni/kni_net.c
+++ b/kernel/linux/kni/kni_net.c
@@ -291,15 +291,15 @@ kni_net_tx(struct sk_buff *skb, struct net_device *dev)
 
 	/* Free skb and update statistics */
 	dev_kfree_skb(skb);
-	kni->stats.tx_bytes += len;
-	kni->stats.tx_packets++;
+	dev->stats.tx_bytes += len;
+	dev->stats.tx_packets++;
 
 	return NETDEV_TX_OK;
 
 drop:
 	/* Free skb and update statistics */
 	dev_kfree_skb(skb);
-	kni->stats.tx_dropped++;
+	dev->stats.tx_dropped++;
 
 	return NETDEV_TX_OK;
 }
@@ -343,7 +343,7 @@ kni_net_rx_normal(struct kni_dev *kni)
 		skb = netdev_alloc_skb(dev, len);
 		if (!skb) {
 			/* Update statistics */
-			kni->stats.rx_dropped++;
+			dev->stats.rx_dropped++;
 			continue;
 		}
 
@@ -372,8 +372,8 @@ kni_net_rx_normal(struct kni_dev *kni)
 		netif_rx_ni(skb);
 
 		/* Update statistics */
-		kni->stats.rx_bytes += len;
-		kni->stats.rx_packets++;
+		dev->stats.rx_bytes += len;
+		dev->stats.rx_packets++;
 	}
 
 	/* Burst enqueue mbufs into free_q */
@@ -396,6 +396,7 @@ kni_net_rx_lo_fifo(struct kni_dev *kni)
 	void *data_kva;
 	struct rte_kni_mbuf *alloc_kva;
 	void *alloc_data_kva;
+	struct net_device *dev = kni->net_dev;
 
 	/* Get the number of entries in rx_q */
 	num_rq = kni_fifo_count(kni->rx_q);
@@ -443,8 +444,8 @@ kni_net_rx_lo_fifo(struct kni_dev *kni)
 			alloc_kva->pkt_len = len;
 			alloc_kva->data_len = len;
 
-			kni->stats.tx_bytes += len;
-			kni->stats.rx_bytes += len;
+			dev->stats.tx_bytes += len;
+			dev->stats.rx_bytes += len;
 		}
 
 		/* Burst enqueue mbufs into tx_q */
@@ -464,8 +465,8 @@ kni_net_rx_lo_fifo(struct kni_dev *kni)
 	 * Update statistic, and enqueue/dequeue failure is impossible,
 	 * as all queues are checked at first.
 	 */
-	kni->stats.tx_packets += num;
-	kni->stats.rx_packets += num;
+	dev->stats.tx_packets += num;
+	dev->stats.rx_packets += num;
 }
 
 /*
@@ -518,7 +519,7 @@ kni_net_rx_lo_fifo_skb(struct kni_dev *kni)
 		/* Simulate real usage, allocate/copy skb twice */
 		skb = netdev_alloc_skb(dev, len);
 		if (skb == NULL) {
-			kni->stats.rx_dropped++;
+			dev->stats.rx_dropped++;
 			continue;
 		}
 
@@ -542,8 +543,8 @@ kni_net_rx_lo_fifo_skb(struct kni_dev *kni)
 
 		skb->ip_summed = CHECKSUM_UNNECESSARY;
 
-		kni->stats.rx_bytes += len;
-		kni->stats.rx_packets++;
+		dev->stats.rx_bytes += len;
+		dev->stats.rx_packets++;
 
 		/* call tx interface */
 		kni_net_tx(skb, dev);
@@ -573,12 +574,10 @@ kni_net_rx(struct kni_dev *kni)
 static void
 kni_net_tx_timeout(struct net_device *dev)
 {
-	struct kni_dev *kni = netdev_priv(dev);
-
 	pr_debug("Transmit timeout at %ld, latency %ld\n", jiffies,
 			jiffies - dev_trans_start(dev));
 
-	kni->stats.tx_errors++;
+	dev->stats.tx_errors++;
 	netif_wake_queue(dev);
 }
 
@@ -627,17 +626,6 @@ kni_net_poll_resp(struct kni_dev *kni)
 		wake_up_interruptible(&kni->wq);
 }
 
-/*
- * Return statistics to the caller
- */
-static struct net_device_stats *
-kni_net_stats(struct net_device *dev)
-{
-	struct kni_dev *kni = netdev_priv(dev);
-
-	return &kni->stats;
-}
-
 /*
  *  Fill the eth header
  */
@@ -730,7 +718,6 @@ static const struct net_device_ops kni_net_netdev_ops = {
 	.ndo_change_rx_flags = kni_net_set_promiscusity,
 	.ndo_start_xmit = kni_net_tx,
 	.ndo_change_mtu = kni_net_change_mtu,
-	.ndo_get_stats = kni_net_stats,
 	.ndo_tx_timeout = kni_net_tx_timeout,
 	.ndo_set_mac_address = kni_net_set_mac,
 #ifdef HAVE_CHANGE_CARRIER_CB
-- 
2.20.1


^ permalink raw reply	[flat|nested] 63+ messages in thread

* [dpdk-dev] [PATCH v4 4/8] kni: drop unused fields
  2019-06-19 18:59 ` [dpdk-dev] [PATCH v4 0/8] kni: fixes and cleanups Stephen Hemminger
                     ` (2 preceding siblings ...)
  2019-06-19 18:59   ` [dpdk-dev] [PATCH v4 3/8] kni: don't keep stats in kni_net Stephen Hemminger
@ 2019-06-19 18:59   ` Stephen Hemminger
  2019-06-19 18:59   ` [dpdk-dev] [PATCH v4 5/8] kni: use proper type for kni fifo's Stephen Hemminger
                     ` (3 subsequent siblings)
  7 siblings, 0 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-19 18:59 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, Stephen Hemminger

From: Stephen Hemminger <sthemmin@microsoft.com>

The kni net structure only exists in driver no API/ABI.
Several fields were either totally unused or set and never used.

The fields in dev_info do need to stay in the ABI but
kernel can ignore them.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

kni: drop unused status element

Yet another ethtool leftover.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 kernel/linux/kni/kni_dev.h  | 5 -----
 kernel/linux/kni/kni_misc.c | 1 -
 2 files changed, 6 deletions(-)

diff --git a/kernel/linux/kni/kni_dev.h b/kernel/linux/kni/kni_dev.h
index 21e4b0d92368..f3e6c3ca4efa 100644
--- a/kernel/linux/kni/kni_dev.h
+++ b/kernel/linux/kni/kni_dev.h
@@ -39,8 +39,6 @@ struct kni_dev {
 	/* kni list */
 	struct list_head list;
 
-	int status;
-	uint16_t group_id;           /* Group ID of a group of KNI devices */
 	uint32_t core_id;            /* Core ID to bind */
 	char name[RTE_KNI_NAMESIZE]; /* Network device name */
 	struct task_struct *pthread;
@@ -79,9 +77,6 @@ struct kni_dev {
 	/* mbuf size */
 	uint32_t mbuf_size;
 
-	/* synchro for request processing */
-	unsigned long synchro;
-
 	/* buffers */
 	void *pa[MBUF_BURST_SZ];
 	void *va[MBUF_BURST_SZ];
diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c
index 1fc5eeb9c8ca..b59cf24c2184 100644
--- a/kernel/linux/kni/kni_misc.c
+++ b/kernel/linux/kni/kni_misc.c
@@ -346,7 +346,6 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num,
 	kni = netdev_priv(net_dev);
 
 	kni->net_dev = net_dev;
-	kni->group_id = dev_info.group_id;
 	kni->core_id = dev_info.core_id;
 	strncpy(kni->name, dev_info.name, RTE_KNI_NAMESIZE);
 
-- 
2.20.1


^ permalink raw reply	[flat|nested] 63+ messages in thread

* [dpdk-dev] [PATCH v4 5/8] kni: use proper type for kni fifo's
  2019-06-19 18:59 ` [dpdk-dev] [PATCH v4 0/8] kni: fixes and cleanups Stephen Hemminger
                     ` (3 preceding siblings ...)
  2019-06-19 18:59   ` [dpdk-dev] [PATCH v4 4/8] kni: drop unused fields Stephen Hemminger
@ 2019-06-19 18:59   ` Stephen Hemminger
  2019-06-19 18:59   ` [dpdk-dev] [PATCH v4 6/8] kni: return -EFAULT if copy_from_user fails Stephen Hemminger
                     ` (2 subsequent siblings)
  7 siblings, 0 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-19 18:59 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, Stephen Hemminger

From: Stephen Hemminger <sthemmin@microsoft.com>

Using void * instead of proper type is unsafe practice.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 kernel/linux/kni/kni_dev.h | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/kernel/linux/kni/kni_dev.h b/kernel/linux/kni/kni_dev.h
index f3e6c3ca4efa..ceba5f73c1d9 100644
--- a/kernel/linux/kni/kni_dev.h
+++ b/kernel/linux/kni/kni_dev.h
@@ -51,22 +51,22 @@ struct kni_dev {
 	struct net_device *net_dev;
 
 	/* queue for packets to be sent out */
-	void *tx_q;
+	struct rte_kni_fifo *tx_q;
 
 	/* queue for the packets received */
-	void *rx_q;
+	struct rte_kni_fifo *rx_q;
 
 	/* queue for the allocated mbufs those can be used to save sk buffs */
-	void *alloc_q;
+	struct rte_kni_fifo *alloc_q;
 
 	/* free queue for the mbufs to be freed */
-	void *free_q;
+	struct rte_kni_fifo *free_q;
 
 	/* request queue */
-	void *req_q;
+	struct rte_kni_fifo *req_q;
 
 	/* response queue */
-	void *resp_q;
+	struct rte_kni_fifo *resp_q;
 
 	void *sync_kva;
 	void *sync_va;
-- 
2.20.1


^ permalink raw reply	[flat|nested] 63+ messages in thread

* [dpdk-dev] [PATCH v4 6/8] kni: return -EFAULT if copy_from_user fails
  2019-06-19 18:59 ` [dpdk-dev] [PATCH v4 0/8] kni: fixes and cleanups Stephen Hemminger
                     ` (4 preceding siblings ...)
  2019-06-19 18:59   ` [dpdk-dev] [PATCH v4 5/8] kni: use proper type for kni fifo's Stephen Hemminger
@ 2019-06-19 18:59   ` Stephen Hemminger
  2019-06-19 18:59   ` [dpdk-dev] [PATCH v4 7/8] doc: update KNI documentation Stephen Hemminger
  2019-06-19 18:59   ` [dpdk-dev] [PATCH v4 8/8] kni: fix style issues Stephen Hemminger
  7 siblings, 0 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-19 18:59 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, Stephen Hemminger

From: Stephen Hemminger <sthemmin@microsoft.com>

The correct thing to return if user gives a bad data
is to return -EFAULT. Logging is also discouraged because
it could be used as a DoS attack.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 kernel/linux/kni/kni_misc.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c
index b59cf24c2184..be45f823408f 100644
--- a/kernel/linux/kni/kni_misc.c
+++ b/kernel/linux/kni/kni_misc.c
@@ -301,11 +301,8 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num,
 		return -EINVAL;
 
 	/* Copy kni info from user space */
-	ret = copy_from_user(&dev_info, (void *)ioctl_param, sizeof(dev_info));
-	if (ret) {
-		pr_err("copy_from_user in kni_ioctl_create");
-		return -EIO;
-	}
+	if (copy_from_user(&dev_info, (void *)ioctl_param, sizeof(dev_info)))
+		return -EFAULT;
 
 	/* Check if name is zero-ended */
 	if (strnlen(dev_info.name, sizeof(dev_info.name)) == sizeof(dev_info.name)) {
@@ -427,15 +424,12 @@ kni_ioctl_release(struct net *net, uint32_t ioctl_num,
 	if (_IOC_SIZE(ioctl_num) > sizeof(dev_info))
 		return -EINVAL;
 
-	ret = copy_from_user(&dev_info, (void *)ioctl_param, sizeof(dev_info));
-	if (ret) {
-		pr_err("copy_from_user in kni_ioctl_release");
-		return -EIO;
-	}
+	if (copy_from_user(&dev_info, (void *)ioctl_param, sizeof(dev_info)))
+		return -EFAULT;
 
 	/* Release the network device according to its name */
 	if (strlen(dev_info.name) == 0)
-		return ret;
+		return -EINVAL;
 
 	down_write(&knet->kni_list_lock);
 	list_for_each_entry_safe(dev, n, &knet->kni_list_head, list) {
-- 
2.20.1


^ permalink raw reply	[flat|nested] 63+ messages in thread

* [dpdk-dev] [PATCH v4 7/8] doc: update KNI documentation
  2019-06-19 18:59 ` [dpdk-dev] [PATCH v4 0/8] kni: fixes and cleanups Stephen Hemminger
                     ` (5 preceding siblings ...)
  2019-06-19 18:59   ` [dpdk-dev] [PATCH v4 6/8] kni: return -EFAULT if copy_from_user fails Stephen Hemminger
@ 2019-06-19 18:59   ` Stephen Hemminger
  2019-06-19 18:59   ` [dpdk-dev] [PATCH v4 8/8] kni: fix style issues Stephen Hemminger
  7 siblings, 0 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-19 18:59 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, Stephen Hemminger

From: Stephen Hemminger <sthemmin@microsoft.com>

Make the KNI documentation reflect modern kernel networking.
Ifconfig has been superseded by iproute2 for 15 years.
Iproute2 is well maintained, supports current feature set.

Ethtool is no longer supported by KNI.
Tshark is a better replacement for tcpdump.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 .../sample_app_ug/kernel_nic_interface.rst     | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/doc/guides/sample_app_ug/kernel_nic_interface.rst b/doc/guides/sample_app_ug/kernel_nic_interface.rst
index a7e549d5213a..422bd8c98465 100644
--- a/doc/guides/sample_app_ug/kernel_nic_interface.rst
+++ b/doc/guides/sample_app_ug/kernel_nic_interface.rst
@@ -21,14 +21,14 @@ The FIFO queues contain pointers to data packets in the DPDK. This:
 
 *   Provides a faster mechanism to interface with the kernel net stack and eliminates system calls
 
-*   Facilitates the DPDK using standard Linux* userspace net tools (tcpdump, ftp, and so on)
+*   Facilitates the DPDK using standard Linux* userspace net tools (tshark, rsync, and so on)
 
 *   Eliminate the copy_to_user and copy_from_user operations on packets.
 
 The Kernel NIC Interface sample application is a simple example that demonstrates the use
 of the DPDK to create a path for packets to go through the Linux* kernel.
 This is done by creating one or more kernel net devices for each of the DPDK ports.
-The application allows the use of standard Linux tools (ethtool, ifconfig, tcpdump) with the DPDK ports and
+The application allows the use of standard Linux tools (iproute, tshark) with the DPDK ports and
 also the exchange of packets between the DPDK application and the Linux* kernel.
 
 The Kernel NIC Interface sample application requires that the
@@ -220,13 +220,13 @@ Enable KNI interface and assign an IP address:
 
 .. code-block:: console
 
-    # ifconfig vEth0_0 192.168.0.1
+    # ip addr add dev vEth0_0 192.168.0.1
 
 Show KNI interface configuration and statistics:
 
 .. code-block:: console
 
-    # ifconfig vEth0_0
+    # ip -s -d addr show vEth0_0
 
 The user can also check and reset the packet statistics inside the ``kni``
 application by sending the app the USR1 and USR2 signals:
@@ -234,16 +234,16 @@ application by sending the app the USR1 and USR2 signals:
 .. code-block:: console
 
     # Print statistics
-    # kill -SIGUSR1 `pidof kni`
+    # pkill -USR1 kni
 
     # Zero statistics
-    # kill -SIGUSR2 `pidof kni`
+    # pkill -USR2 kni
 
 Dump network traffic:
 
 .. code-block:: console
 
-    # tcpdump -i vEth0_0
+    # tshark -n -i vEth0_0
 
 The normal Linux commands can also be used to change the MAC address and
 MTU size used by the physical NIC which corresponds to the KNI interface.
@@ -254,13 +254,13 @@ Change the MAC address:
 
 .. code-block:: console
 
-    # ifconfig vEth0_0 hw ether 0C:01:02:03:04:08
+    # ip link set dev vEth0_0 lladdr 0C:01:02:03:04:08
 
 Change the MTU size:
 
 .. code-block:: console
 
-    # ifconfig vEth0_0 mtu 1450
+    # ip link set dev vEth0_0 mtu 1450
 
 When the ``kni`` application is closed, all the KNI interfaces are deleted
 from the Linux kernel.
-- 
2.20.1


^ permalink raw reply	[flat|nested] 63+ messages in thread

* [dpdk-dev] [PATCH v4 8/8] kni: fix style issues
  2019-06-19 18:59 ` [dpdk-dev] [PATCH v4 0/8] kni: fixes and cleanups Stephen Hemminger
                     ` (6 preceding siblings ...)
  2019-06-19 18:59   ` [dpdk-dev] [PATCH v4 7/8] doc: update KNI documentation Stephen Hemminger
@ 2019-06-19 18:59   ` Stephen Hemminger
  7 siblings, 0 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-19 18:59 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, Stephen Hemminger

From: Stephen Hemminger <sthemmin@microsoft.com>

rte_kni does not follow standard style rules.
Noticed some extra \ line continuation etc.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_kni/rte_kni.c | 38 +++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/lib/librte_kni/rte_kni.c b/lib/librte_kni/rte_kni.c
index e29d0cc7df3c..9b6acc382fc3 100644
--- a/lib/librte_kni/rte_kni.c
+++ b/lib/librte_kni/rte_kni.c
@@ -59,7 +59,7 @@ struct rte_kni {
 	uint16_t group_id;                  /**< Group ID of KNI devices */
 	uint32_t slot_id;                   /**< KNI pool slot ID */
 	struct rte_mempool *pktmbuf_pool;   /**< pkt mbuf mempool */
-	unsigned mbuf_size;                 /**< mbuf size */
+	unsigned int mbuf_size;                 /**< mbuf size */
 
 	const struct rte_memzone *m_tx_q;   /**< TX queue memzone */
 	const struct rte_memzone *m_rx_q;   /**< RX queue memzone */
@@ -78,7 +78,7 @@ struct rte_kni {
 	/* For request & response */
 	struct rte_kni_fifo *req_q;         /**< Request queue */
 	struct rte_kni_fifo *resp_q;        /**< Response queue */
-	void * sync_addr;                   /**< Req/Resp Mem address */
+	void *sync_addr;                   /**< Req/Resp Mem address */
 
 	struct rte_kni_ops ops;             /**< operations for request */
 };
@@ -473,7 +473,7 @@ kni_config_promiscusity(uint16_t port_id, uint8_t to_on)
 int
 rte_kni_handle_request(struct rte_kni *kni)
 {
-	unsigned ret;
+	unsigned int ret;
 	struct rte_kni_request *req = NULL;
 
 	if (kni == NULL)
@@ -498,8 +498,8 @@ rte_kni_handle_request(struct rte_kni *kni)
 		break;
 	case RTE_KNI_REQ_CFG_NETWORK_IF: /* Set network interface up/down */
 		if (kni->ops.config_network_if)
-			req->result = kni->ops.config_network_if(\
-					kni->ops.port_id, req->if_up);
+			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)
@@ -534,7 +534,7 @@ rte_kni_handle_request(struct rte_kni *kni)
 }
 
 unsigned
-rte_kni_tx_burst(struct rte_kni *kni, struct rte_mbuf **mbufs, unsigned num)
+rte_kni_tx_burst(struct rte_kni *kni, struct rte_mbuf **mbufs, unsigned int num)
 {
 	void *phy_mbufs[num];
 	unsigned int ret;
@@ -552,9 +552,9 @@ rte_kni_tx_burst(struct rte_kni *kni, struct rte_mbuf **mbufs, unsigned num)
 }
 
 unsigned
-rte_kni_rx_burst(struct rte_kni *kni, struct rte_mbuf **mbufs, unsigned num)
+rte_kni_rx_burst(struct rte_kni *kni, struct rte_mbuf **mbufs, unsigned int num)
 {
-	unsigned ret = kni_fifo_get(kni->tx_q, (void **)mbufs, num);
+	unsigned int ret = kni_fifo_get(kni->tx_q, (void **)mbufs, num);
 
 	/* If buffers removed, allocate mbufs and then put them into alloc_q */
 	if (ret)
@@ -605,7 +605,7 @@ kni_allocate_mbufs(struct rte_kni *kni)
 		return;
 	}
 
-	allocq_free = (kni->alloc_q->read - kni->alloc_q->write - 1) \
+	allocq_free = (kni->alloc_q->read - kni->alloc_q->write - 1)
 			& (MAX_MBUF_BURST_NUM - 1);
 	for (i = 0; i < allocq_free; i++) {
 		pkts[i] = rte_pktmbuf_alloc(kni->pktmbuf_pool);
@@ -659,35 +659,35 @@ static enum kni_ops_status
 kni_check_request_register(struct rte_kni_ops *ops)
 {
 	/* check if KNI request ops has been registered*/
-	if( NULL == ops )
+	if (ops == NULL)
 		return KNI_REQ_NO_REGISTER;
 
-	if ((ops->change_mtu == NULL)
-		&& (ops->config_network_if == NULL)
-		&& (ops->config_mac_address == NULL)
-		&& (ops->config_promiscusity == NULL))
+	if (ops->change_mtu == NULL
+	    && ops->config_network_if == NULL
+	    && ops->config_mac_address == NULL
+	    && ops->config_promiscusity == NULL)
 		return KNI_REQ_NO_REGISTER;
 
 	return KNI_REQ_REGISTERED;
 }
 
 int
-rte_kni_register_handlers(struct rte_kni *kni,struct rte_kni_ops *ops)
+rte_kni_register_handlers(struct rte_kni *kni, struct rte_kni_ops *ops)
 {
 	enum kni_ops_status req_status;
 
-	if (NULL == ops) {
+	if (ops == NULL) {
 		RTE_LOG(ERR, KNI, "Invalid KNI request operation.\n");
 		return -1;
 	}
 
-	if (NULL == kni) {
+	if (kni == NULL) {
 		RTE_LOG(ERR, KNI, "Invalid kni info.\n");
 		return -1;
 	}
 
 	req_status = kni_check_request_register(&kni->ops);
-	if ( KNI_REQ_REGISTERED == req_status) {
+	if (req_status == KNI_REQ_REGISTERED) {
 		RTE_LOG(ERR, KNI, "The KNI request operation has already registered.\n");
 		return -1;
 	}
@@ -699,7 +699,7 @@ rte_kni_register_handlers(struct rte_kni *kni,struct rte_kni_ops *ops)
 int
 rte_kni_unregister_handlers(struct rte_kni *kni)
 {
-	if (NULL == kni) {
+	if (kni == NULL) {
 		RTE_LOG(ERR, KNI, "Invalid kni info.\n");
 		return -1;
 	}
-- 
2.20.1


^ permalink raw reply	[flat|nested] 63+ messages in thread

* [dpdk-dev] [PATCH v5 0/9] kni: fixes and cleanups
  2019-06-10 17:51 [dpdk-dev] [PATCH v2 0/7] kni: cleanups and improvements Stephen Hemminger
                   ` (10 preceding siblings ...)
  2019-06-19 18:59 ` [dpdk-dev] [PATCH v4 0/8] kni: fixes and cleanups Stephen Hemminger
@ 2019-06-20 19:20 ` Stephen Hemminger
  2019-06-20 19:20   ` [dpdk-dev] [PATCH v5 1/9] kni: don't need stubs for rx_mode or ioctl Stephen Hemminger
                     ` (8 more replies)
  2019-06-24 16:47 ` [dpdk-dev] [PATCH v6 0/9] kni: fixes and cleanups Stephen Hemminger
  12 siblings, 9 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-20 19:20 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

While testing KNI with netvsc, saw lots of places more code
could be safely removed from KNI kernel driver.

v5 - add minimal ethtool, fix checkpath author complaints
v4 - add more style fixes
v3 - rebase to current master, add style fix patch
v2 - get rid of unnecessary padding, combine the unused field patches

Stephen Hemminger (9):
  kni: don't need stubs for rx_mode or ioctl
  kni: use netdev_alloc_skb
  kni: don't keep stats in kni_net
  kni: drop unused fields
  kni: use proper type for kni fifo's
  kni: return -EFAULT if copy_from_user fails
  doc: update KNI documentation
  kni: fix style issues
  kni: add minimal ethtool

 .../sample_app_ug/kernel_nic_interface.rst    |  18 ++--
 kernel/linux/kni/kni_dev.h                    |  20 ++--
 kernel/linux/kni/kni_misc.c                   |  18 ++--
 kernel/linux/kni/kni_net.c                    | 100 +++++++-----------
 lib/librte_kni/rte_kni.c                      |  38 +++----
 5 files changed, 78 insertions(+), 116 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 63+ messages in thread

* [dpdk-dev] [PATCH v5 1/9] kni: don't need stubs for rx_mode or ioctl
  2019-06-20 19:20 ` [dpdk-dev] [PATCH v5 0/9] kni: fixes and cleanups Stephen Hemminger
@ 2019-06-20 19:20   ` Stephen Hemminger
  2019-06-20 19:20   ` [dpdk-dev] [PATCH v5 2/9] kni: use netdev_alloc_skb Stephen Hemminger
                     ` (7 subsequent siblings)
  8 siblings, 0 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-20 19:20 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

The netdev subsystem already handles case where
network sevice does not support ioctl.

If device has no rx_mode hook it is not called.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 kernel/linux/kni/kni_net.c | 19 -------------------
 1 file changed, 19 deletions(-)

diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c
index ad8365877cda..c86337d099ab 100644
--- a/kernel/linux/kni/kni_net.c
+++ b/kernel/linux/kni/kni_net.c
@@ -593,23 +593,6 @@ kni_net_tx_timeout(struct net_device *dev)
 	netif_wake_queue(dev);
 }
 
-/*
- * Ioctl commands
- */
-static int
-kni_net_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
-{
-	pr_debug("kni_net_ioctl group:%d cmd:%d\n",
-		((struct kni_dev *)netdev_priv(dev))->group_id, cmd);
-
-	return -EOPNOTSUPP;
-}
-
-static void
-kni_net_set_rx_mode(struct net_device *dev)
-{
-}
-
 static int
 kni_net_change_mtu(struct net_device *dev, int new_mtu)
 {
@@ -758,8 +741,6 @@ static const struct net_device_ops kni_net_netdev_ops = {
 	.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,
-	.ndo_set_rx_mode = kni_net_set_rx_mode,
 	.ndo_get_stats = kni_net_stats,
 	.ndo_tx_timeout = kni_net_tx_timeout,
 	.ndo_set_mac_address = kni_net_set_mac,
-- 
2.20.1


^ permalink raw reply	[flat|nested] 63+ messages in thread

* [dpdk-dev] [PATCH v5 2/9] kni: use netdev_alloc_skb
  2019-06-20 19:20 ` [dpdk-dev] [PATCH v5 0/9] kni: fixes and cleanups Stephen Hemminger
  2019-06-20 19:20   ` [dpdk-dev] [PATCH v5 1/9] kni: don't need stubs for rx_mode or ioctl Stephen Hemminger
@ 2019-06-20 19:20   ` Stephen Hemminger
  2019-06-20 19:20   ` [dpdk-dev] [PATCH v5 3/9] kni: don't keep stats in kni_net Stephen Hemminger
                     ` (6 subsequent siblings)
  8 siblings, 0 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-20 19:20 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

netdev_alloc_skb is optimized to any alignment or setup
of skb->dev that is required. The kernel has chosen to not pad
packets on x86 (for many years), because it is faster.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 kernel/linux/kni/kni_net.c | 17 +++--------------
 1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c
index c86337d099ab..cce5e7eb991f 100644
--- a/kernel/linux/kni/kni_net.c
+++ b/kernel/linux/kni/kni_net.c
@@ -340,16 +340,13 @@ kni_net_rx_normal(struct kni_dev *kni)
 		data_kva = kva2data_kva(kva);
 		kni->va[i] = pa2va(kni->pa[i], kva);
 
-		skb = dev_alloc_skb(len + 2);
+		skb = netdev_alloc_skb(dev, len);
 		if (!skb) {
 			/* Update statistics */
 			kni->stats.rx_dropped++;
 			continue;
 		}
 
-		/* Align IP on 16B boundary */
-		skb_reserve(skb, 2);
-
 		if (kva->nb_segs == 1) {
 			memcpy(skb_put(skb, len), data_kva, len);
 		} else {
@@ -368,7 +365,6 @@ kni_net_rx_normal(struct kni_dev *kni)
 			}
 		}
 
-		skb->dev = dev;
 		skb->protocol = eth_type_trans(skb, dev);
 		skb->ip_summed = CHECKSUM_UNNECESSARY;
 
@@ -512,26 +508,20 @@ kni_net_rx_lo_fifo_skb(struct kni_dev *kni)
 		data_kva = kva2data_kva(kva);
 		kni->va[i] = pa2va(kni->pa[i], kva);
 
-		skb = dev_alloc_skb(len + 2);
+		skb = netdev_alloc_skb(dev, len);
 		if (skb) {
-			/* Align IP on 16B boundary */
-			skb_reserve(skb, 2);
 			memcpy(skb_put(skb, len), data_kva, len);
-			skb->dev = dev;
 			skb->ip_summed = CHECKSUM_UNNECESSARY;
 			dev_kfree_skb(skb);
 		}
 
 		/* Simulate real usage, allocate/copy skb twice */
-		skb = dev_alloc_skb(len + 2);
+		skb = netdev_alloc_skb(dev, len);
 		if (skb == NULL) {
 			kni->stats.rx_dropped++;
 			continue;
 		}
 
-		/* Align IP on 16B boundary */
-		skb_reserve(skb, 2);
-
 		if (kva->nb_segs == 1) {
 			memcpy(skb_put(skb, len), data_kva, len);
 		} else {
@@ -550,7 +540,6 @@ kni_net_rx_lo_fifo_skb(struct kni_dev *kni)
 			}
 		}
 
-		skb->dev = dev;
 		skb->ip_summed = CHECKSUM_UNNECESSARY;
 
 		kni->stats.rx_bytes += len;
-- 
2.20.1


^ permalink raw reply	[flat|nested] 63+ messages in thread

* [dpdk-dev] [PATCH v5 3/9] kni: don't keep stats in kni_net
  2019-06-20 19:20 ` [dpdk-dev] [PATCH v5 0/9] kni: fixes and cleanups Stephen Hemminger
  2019-06-20 19:20   ` [dpdk-dev] [PATCH v5 1/9] kni: don't need stubs for rx_mode or ioctl Stephen Hemminger
  2019-06-20 19:20   ` [dpdk-dev] [PATCH v5 2/9] kni: use netdev_alloc_skb Stephen Hemminger
@ 2019-06-20 19:20   ` Stephen Hemminger
  2019-06-20 19:20   ` [dpdk-dev] [PATCH v5 4/9] kni: drop unused fields Stephen Hemminger
                     ` (5 subsequent siblings)
  8 siblings, 0 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-20 19:20 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

Since kernel 2.6.28 the network subsystem has provided
dev->stats for devices to use statistics handling and is the
default if no ndo_get_stats is provided.

This allow allows for 64 bit (rather than just 32 bit)
statistics with KNI.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 kernel/linux/kni/kni_dev.h |  1 -
 kernel/linux/kni/kni_net.c | 43 +++++++++++++-------------------------
 2 files changed, 15 insertions(+), 29 deletions(-)

diff --git a/kernel/linux/kni/kni_dev.h b/kernel/linux/kni/kni_dev.h
index d57bce647e4a..21e4b0d92368 100644
--- a/kernel/linux/kni/kni_dev.h
+++ b/kernel/linux/kni/kni_dev.h
@@ -39,7 +39,6 @@ struct kni_dev {
 	/* kni list */
 	struct list_head list;
 
-	struct net_device_stats stats;
 	int status;
 	uint16_t group_id;           /* Group ID of a group of KNI devices */
 	uint32_t core_id;            /* Core ID to bind */
diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c
index cce5e7eb991f..06310fec57bb 100644
--- a/kernel/linux/kni/kni_net.c
+++ b/kernel/linux/kni/kni_net.c
@@ -291,15 +291,15 @@ kni_net_tx(struct sk_buff *skb, struct net_device *dev)
 
 	/* Free skb and update statistics */
 	dev_kfree_skb(skb);
-	kni->stats.tx_bytes += len;
-	kni->stats.tx_packets++;
+	dev->stats.tx_bytes += len;
+	dev->stats.tx_packets++;
 
 	return NETDEV_TX_OK;
 
 drop:
 	/* Free skb and update statistics */
 	dev_kfree_skb(skb);
-	kni->stats.tx_dropped++;
+	dev->stats.tx_dropped++;
 
 	return NETDEV_TX_OK;
 }
@@ -343,7 +343,7 @@ kni_net_rx_normal(struct kni_dev *kni)
 		skb = netdev_alloc_skb(dev, len);
 		if (!skb) {
 			/* Update statistics */
-			kni->stats.rx_dropped++;
+			dev->stats.rx_dropped++;
 			continue;
 		}
 
@@ -372,8 +372,8 @@ kni_net_rx_normal(struct kni_dev *kni)
 		netif_rx_ni(skb);
 
 		/* Update statistics */
-		kni->stats.rx_bytes += len;
-		kni->stats.rx_packets++;
+		dev->stats.rx_bytes += len;
+		dev->stats.rx_packets++;
 	}
 
 	/* Burst enqueue mbufs into free_q */
@@ -396,6 +396,7 @@ kni_net_rx_lo_fifo(struct kni_dev *kni)
 	void *data_kva;
 	struct rte_kni_mbuf *alloc_kva;
 	void *alloc_data_kva;
+	struct net_device *dev = kni->net_dev;
 
 	/* Get the number of entries in rx_q */
 	num_rq = kni_fifo_count(kni->rx_q);
@@ -443,8 +444,8 @@ kni_net_rx_lo_fifo(struct kni_dev *kni)
 			alloc_kva->pkt_len = len;
 			alloc_kva->data_len = len;
 
-			kni->stats.tx_bytes += len;
-			kni->stats.rx_bytes += len;
+			dev->stats.tx_bytes += len;
+			dev->stats.rx_bytes += len;
 		}
 
 		/* Burst enqueue mbufs into tx_q */
@@ -464,8 +465,8 @@ kni_net_rx_lo_fifo(struct kni_dev *kni)
 	 * Update statistic, and enqueue/dequeue failure is impossible,
 	 * as all queues are checked at first.
 	 */
-	kni->stats.tx_packets += num;
-	kni->stats.rx_packets += num;
+	dev->stats.tx_packets += num;
+	dev->stats.rx_packets += num;
 }
 
 /*
@@ -518,7 +519,7 @@ kni_net_rx_lo_fifo_skb(struct kni_dev *kni)
 		/* Simulate real usage, allocate/copy skb twice */
 		skb = netdev_alloc_skb(dev, len);
 		if (skb == NULL) {
-			kni->stats.rx_dropped++;
+			dev->stats.rx_dropped++;
 			continue;
 		}
 
@@ -542,8 +543,8 @@ kni_net_rx_lo_fifo_skb(struct kni_dev *kni)
 
 		skb->ip_summed = CHECKSUM_UNNECESSARY;
 
-		kni->stats.rx_bytes += len;
-		kni->stats.rx_packets++;
+		dev->stats.rx_bytes += len;
+		dev->stats.rx_packets++;
 
 		/* call tx interface */
 		kni_net_tx(skb, dev);
@@ -573,12 +574,10 @@ kni_net_rx(struct kni_dev *kni)
 static void
 kni_net_tx_timeout(struct net_device *dev)
 {
-	struct kni_dev *kni = netdev_priv(dev);
-
 	pr_debug("Transmit timeout at %ld, latency %ld\n", jiffies,
 			jiffies - dev_trans_start(dev));
 
-	kni->stats.tx_errors++;
+	dev->stats.tx_errors++;
 	netif_wake_queue(dev);
 }
 
@@ -627,17 +626,6 @@ kni_net_poll_resp(struct kni_dev *kni)
 		wake_up_interruptible(&kni->wq);
 }
 
-/*
- * Return statistics to the caller
- */
-static struct net_device_stats *
-kni_net_stats(struct net_device *dev)
-{
-	struct kni_dev *kni = netdev_priv(dev);
-
-	return &kni->stats;
-}
-
 /*
  *  Fill the eth header
  */
@@ -730,7 +718,6 @@ static const struct net_device_ops kni_net_netdev_ops = {
 	.ndo_change_rx_flags = kni_net_set_promiscusity,
 	.ndo_start_xmit = kni_net_tx,
 	.ndo_change_mtu = kni_net_change_mtu,
-	.ndo_get_stats = kni_net_stats,
 	.ndo_tx_timeout = kni_net_tx_timeout,
 	.ndo_set_mac_address = kni_net_set_mac,
 #ifdef HAVE_CHANGE_CARRIER_CB
-- 
2.20.1


^ permalink raw reply	[flat|nested] 63+ messages in thread

* [dpdk-dev] [PATCH v5 4/9] kni: drop unused fields
  2019-06-20 19:20 ` [dpdk-dev] [PATCH v5 0/9] kni: fixes and cleanups Stephen Hemminger
                     ` (2 preceding siblings ...)
  2019-06-20 19:20   ` [dpdk-dev] [PATCH v5 3/9] kni: don't keep stats in kni_net Stephen Hemminger
@ 2019-06-20 19:20   ` Stephen Hemminger
  2019-06-20 19:20   ` [dpdk-dev] [PATCH v5 5/9] kni: use proper type for kni fifo's Stephen Hemminger
                     ` (4 subsequent siblings)
  8 siblings, 0 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-20 19:20 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

Several fields were either totally unused or set and never used.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 kernel/linux/kni/kni_dev.h  | 5 -----
 kernel/linux/kni/kni_misc.c | 1 -
 2 files changed, 6 deletions(-)

diff --git a/kernel/linux/kni/kni_dev.h b/kernel/linux/kni/kni_dev.h
index 21e4b0d92368..f3e6c3ca4efa 100644
--- a/kernel/linux/kni/kni_dev.h
+++ b/kernel/linux/kni/kni_dev.h
@@ -39,8 +39,6 @@ struct kni_dev {
 	/* kni list */
 	struct list_head list;
 
-	int status;
-	uint16_t group_id;           /* Group ID of a group of KNI devices */
 	uint32_t core_id;            /* Core ID to bind */
 	char name[RTE_KNI_NAMESIZE]; /* Network device name */
 	struct task_struct *pthread;
@@ -79,9 +77,6 @@ struct kni_dev {
 	/* mbuf size */
 	uint32_t mbuf_size;
 
-	/* synchro for request processing */
-	unsigned long synchro;
-
 	/* buffers */
 	void *pa[MBUF_BURST_SZ];
 	void *va[MBUF_BURST_SZ];
diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c
index 1fc5eeb9c8ca..b59cf24c2184 100644
--- a/kernel/linux/kni/kni_misc.c
+++ b/kernel/linux/kni/kni_misc.c
@@ -346,7 +346,6 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num,
 	kni = netdev_priv(net_dev);
 
 	kni->net_dev = net_dev;
-	kni->group_id = dev_info.group_id;
 	kni->core_id = dev_info.core_id;
 	strncpy(kni->name, dev_info.name, RTE_KNI_NAMESIZE);
 
-- 
2.20.1


^ permalink raw reply	[flat|nested] 63+ messages in thread

* [dpdk-dev] [PATCH v5 5/9] kni: use proper type for kni fifo's
  2019-06-20 19:20 ` [dpdk-dev] [PATCH v5 0/9] kni: fixes and cleanups Stephen Hemminger
                     ` (3 preceding siblings ...)
  2019-06-20 19:20   ` [dpdk-dev] [PATCH v5 4/9] kni: drop unused fields Stephen Hemminger
@ 2019-06-20 19:20   ` Stephen Hemminger
  2019-06-20 19:20   ` [dpdk-dev] [PATCH v5 6/9] kni: return -EFAULT if copy_from_user fails Stephen Hemminger
                     ` (3 subsequent siblings)
  8 siblings, 0 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-20 19:20 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

Using void * instead of proper type is unsafe practice.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 kernel/linux/kni/kni_dev.h | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/kernel/linux/kni/kni_dev.h b/kernel/linux/kni/kni_dev.h
index f3e6c3ca4efa..ceba5f73c1d9 100644
--- a/kernel/linux/kni/kni_dev.h
+++ b/kernel/linux/kni/kni_dev.h
@@ -51,22 +51,22 @@ struct kni_dev {
 	struct net_device *net_dev;
 
 	/* queue for packets to be sent out */
-	void *tx_q;
+	struct rte_kni_fifo *tx_q;
 
 	/* queue for the packets received */
-	void *rx_q;
+	struct rte_kni_fifo *rx_q;
 
 	/* queue for the allocated mbufs those can be used to save sk buffs */
-	void *alloc_q;
+	struct rte_kni_fifo *alloc_q;
 
 	/* free queue for the mbufs to be freed */
-	void *free_q;
+	struct rte_kni_fifo *free_q;
 
 	/* request queue */
-	void *req_q;
+	struct rte_kni_fifo *req_q;
 
 	/* response queue */
-	void *resp_q;
+	struct rte_kni_fifo *resp_q;
 
 	void *sync_kva;
 	void *sync_va;
-- 
2.20.1


^ permalink raw reply	[flat|nested] 63+ messages in thread

* [dpdk-dev] [PATCH v5 6/9] kni: return -EFAULT if copy_from_user fails
  2019-06-20 19:20 ` [dpdk-dev] [PATCH v5 0/9] kni: fixes and cleanups Stephen Hemminger
                     ` (4 preceding siblings ...)
  2019-06-20 19:20   ` [dpdk-dev] [PATCH v5 5/9] kni: use proper type for kni fifo's Stephen Hemminger
@ 2019-06-20 19:20   ` Stephen Hemminger
  2019-06-20 19:20   ` [dpdk-dev] [PATCH v5 7/9] doc: update KNI documentation Stephen Hemminger
                     ` (2 subsequent siblings)
  8 siblings, 0 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-20 19:20 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

The correct thing to return if user gives a bad data
is to return -EFAULT. Logging is also discouraged because
it could be used as a DoS attack.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 kernel/linux/kni/kni_misc.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c
index b59cf24c2184..be45f823408f 100644
--- a/kernel/linux/kni/kni_misc.c
+++ b/kernel/linux/kni/kni_misc.c
@@ -301,11 +301,8 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num,
 		return -EINVAL;
 
 	/* Copy kni info from user space */
-	ret = copy_from_user(&dev_info, (void *)ioctl_param, sizeof(dev_info));
-	if (ret) {
-		pr_err("copy_from_user in kni_ioctl_create");
-		return -EIO;
-	}
+	if (copy_from_user(&dev_info, (void *)ioctl_param, sizeof(dev_info)))
+		return -EFAULT;
 
 	/* Check if name is zero-ended */
 	if (strnlen(dev_info.name, sizeof(dev_info.name)) == sizeof(dev_info.name)) {
@@ -427,15 +424,12 @@ kni_ioctl_release(struct net *net, uint32_t ioctl_num,
 	if (_IOC_SIZE(ioctl_num) > sizeof(dev_info))
 		return -EINVAL;
 
-	ret = copy_from_user(&dev_info, (void *)ioctl_param, sizeof(dev_info));
-	if (ret) {
-		pr_err("copy_from_user in kni_ioctl_release");
-		return -EIO;
-	}
+	if (copy_from_user(&dev_info, (void *)ioctl_param, sizeof(dev_info)))
+		return -EFAULT;
 
 	/* Release the network device according to its name */
 	if (strlen(dev_info.name) == 0)
-		return ret;
+		return -EINVAL;
 
 	down_write(&knet->kni_list_lock);
 	list_for_each_entry_safe(dev, n, &knet->kni_list_head, list) {
-- 
2.20.1


^ permalink raw reply	[flat|nested] 63+ messages in thread

* [dpdk-dev] [PATCH v5 7/9] doc: update KNI documentation
  2019-06-20 19:20 ` [dpdk-dev] [PATCH v5 0/9] kni: fixes and cleanups Stephen Hemminger
                     ` (5 preceding siblings ...)
  2019-06-20 19:20   ` [dpdk-dev] [PATCH v5 6/9] kni: return -EFAULT if copy_from_user fails Stephen Hemminger
@ 2019-06-20 19:20   ` Stephen Hemminger
  2019-06-20 19:20   ` [dpdk-dev] [PATCH v5 8/9] kni: fix style issues Stephen Hemminger
  2019-06-20 19:20   ` [dpdk-dev] [PATCH v5 9/9] kni: add minimal ethtool Stephen Hemminger
  8 siblings, 0 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-20 19:20 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

Make the KNI documentation reflect modern kernel networking.
Ifconfig has been superseded by iproute2 for 15 years.
Iproute2 is well maintained, supports current feature set.

Ethtool is no longer supported by KNI.
Tshark is a better replacement for tcpdump.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 .../sample_app_ug/kernel_nic_interface.rst     | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/doc/guides/sample_app_ug/kernel_nic_interface.rst b/doc/guides/sample_app_ug/kernel_nic_interface.rst
index a7e549d5213a..422bd8c98465 100644
--- a/doc/guides/sample_app_ug/kernel_nic_interface.rst
+++ b/doc/guides/sample_app_ug/kernel_nic_interface.rst
@@ -21,14 +21,14 @@ The FIFO queues contain pointers to data packets in the DPDK. This:
 
 *   Provides a faster mechanism to interface with the kernel net stack and eliminates system calls
 
-*   Facilitates the DPDK using standard Linux* userspace net tools (tcpdump, ftp, and so on)
+*   Facilitates the DPDK using standard Linux* userspace net tools (tshark, rsync, and so on)
 
 *   Eliminate the copy_to_user and copy_from_user operations on packets.
 
 The Kernel NIC Interface sample application is a simple example that demonstrates the use
 of the DPDK to create a path for packets to go through the Linux* kernel.
 This is done by creating one or more kernel net devices for each of the DPDK ports.
-The application allows the use of standard Linux tools (ethtool, ifconfig, tcpdump) with the DPDK ports and
+The application allows the use of standard Linux tools (iproute, tshark) with the DPDK ports and
 also the exchange of packets between the DPDK application and the Linux* kernel.
 
 The Kernel NIC Interface sample application requires that the
@@ -220,13 +220,13 @@ Enable KNI interface and assign an IP address:
 
 .. code-block:: console
 
-    # ifconfig vEth0_0 192.168.0.1
+    # ip addr add dev vEth0_0 192.168.0.1
 
 Show KNI interface configuration and statistics:
 
 .. code-block:: console
 
-    # ifconfig vEth0_0
+    # ip -s -d addr show vEth0_0
 
 The user can also check and reset the packet statistics inside the ``kni``
 application by sending the app the USR1 and USR2 signals:
@@ -234,16 +234,16 @@ application by sending the app the USR1 and USR2 signals:
 .. code-block:: console
 
     # Print statistics
-    # kill -SIGUSR1 `pidof kni`
+    # pkill -USR1 kni
 
     # Zero statistics
-    # kill -SIGUSR2 `pidof kni`
+    # pkill -USR2 kni
 
 Dump network traffic:
 
 .. code-block:: console
 
-    # tcpdump -i vEth0_0
+    # tshark -n -i vEth0_0
 
 The normal Linux commands can also be used to change the MAC address and
 MTU size used by the physical NIC which corresponds to the KNI interface.
@@ -254,13 +254,13 @@ Change the MAC address:
 
 .. code-block:: console
 
-    # ifconfig vEth0_0 hw ether 0C:01:02:03:04:08
+    # ip link set dev vEth0_0 lladdr 0C:01:02:03:04:08
 
 Change the MTU size:
 
 .. code-block:: console
 
-    # ifconfig vEth0_0 mtu 1450
+    # ip link set dev vEth0_0 mtu 1450
 
 When the ``kni`` application is closed, all the KNI interfaces are deleted
 from the Linux kernel.
-- 
2.20.1


^ permalink raw reply	[flat|nested] 63+ messages in thread

* [dpdk-dev] [PATCH v5 8/9] kni: fix style issues
  2019-06-20 19:20 ` [dpdk-dev] [PATCH v5 0/9] kni: fixes and cleanups Stephen Hemminger
                     ` (6 preceding siblings ...)
  2019-06-20 19:20   ` [dpdk-dev] [PATCH v5 7/9] doc: update KNI documentation Stephen Hemminger
@ 2019-06-20 19:20   ` Stephen Hemminger
  2019-06-20 19:20   ` [dpdk-dev] [PATCH v5 9/9] kni: add minimal ethtool Stephen Hemminger
  8 siblings, 0 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-20 19:20 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

rte_kni does not follow standard style rules.
Noticed some extra \ line continuation etc.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 kernel/linux/kni/kni_net.c |  7 ++++---
 lib/librte_kni/rte_kni.c   | 38 +++++++++++++++++++-------------------
 2 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c
index 06310fec57bb..320d51d7fc83 100644
--- a/kernel/linux/kni/kni_net.c
+++ b/kernel/linux/kni/kni_net.c
@@ -401,7 +401,7 @@ kni_net_rx_lo_fifo(struct kni_dev *kni)
 	/* Get the number of entries in rx_q */
 	num_rq = kni_fifo_count(kni->rx_q);
 
-	/* Get the number of free entrie in tx_q */
+	/* Get the number of free entries in tx_q */
 	num_tq = kni_fifo_free_count(kni->tx_q);
 
 	/* Get the number of entries in alloc_q */
@@ -755,6 +755,7 @@ kni_net_config_lo_mode(char *lo_str)
 	} else if (!strcmp(lo_str, "lo_mode_fifo_skb")) {
 		pr_debug("loopback mode=lo_mode_fifo_skb enabled");
 		kni_net_rx_func = kni_net_rx_lo_fifo_skb;
-	} else
-		pr_debug("Incognizant parameter, loopback disabled");
+	} else {
+		pr_debug("Unknown loopback parameter, disabled");
+	}
 }
diff --git a/lib/librte_kni/rte_kni.c b/lib/librte_kni/rte_kni.c
index e29d0cc7df3c..9b6acc382fc3 100644
--- a/lib/librte_kni/rte_kni.c
+++ b/lib/librte_kni/rte_kni.c
@@ -59,7 +59,7 @@ struct rte_kni {
 	uint16_t group_id;                  /**< Group ID of KNI devices */
 	uint32_t slot_id;                   /**< KNI pool slot ID */
 	struct rte_mempool *pktmbuf_pool;   /**< pkt mbuf mempool */
-	unsigned mbuf_size;                 /**< mbuf size */
+	unsigned int mbuf_size;                 /**< mbuf size */
 
 	const struct rte_memzone *m_tx_q;   /**< TX queue memzone */
 	const struct rte_memzone *m_rx_q;   /**< RX queue memzone */
@@ -78,7 +78,7 @@ struct rte_kni {
 	/* For request & response */
 	struct rte_kni_fifo *req_q;         /**< Request queue */
 	struct rte_kni_fifo *resp_q;        /**< Response queue */
-	void * sync_addr;                   /**< Req/Resp Mem address */
+	void *sync_addr;                   /**< Req/Resp Mem address */
 
 	struct rte_kni_ops ops;             /**< operations for request */
 };
@@ -473,7 +473,7 @@ kni_config_promiscusity(uint16_t port_id, uint8_t to_on)
 int
 rte_kni_handle_request(struct rte_kni *kni)
 {
-	unsigned ret;
+	unsigned int ret;
 	struct rte_kni_request *req = NULL;
 
 	if (kni == NULL)
@@ -498,8 +498,8 @@ rte_kni_handle_request(struct rte_kni *kni)
 		break;
 	case RTE_KNI_REQ_CFG_NETWORK_IF: /* Set network interface up/down */
 		if (kni->ops.config_network_if)
-			req->result = kni->ops.config_network_if(\
-					kni->ops.port_id, req->if_up);
+			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)
@@ -534,7 +534,7 @@ rte_kni_handle_request(struct rte_kni *kni)
 }
 
 unsigned
-rte_kni_tx_burst(struct rte_kni *kni, struct rte_mbuf **mbufs, unsigned num)
+rte_kni_tx_burst(struct rte_kni *kni, struct rte_mbuf **mbufs, unsigned int num)
 {
 	void *phy_mbufs[num];
 	unsigned int ret;
@@ -552,9 +552,9 @@ rte_kni_tx_burst(struct rte_kni *kni, struct rte_mbuf **mbufs, unsigned num)
 }
 
 unsigned
-rte_kni_rx_burst(struct rte_kni *kni, struct rte_mbuf **mbufs, unsigned num)
+rte_kni_rx_burst(struct rte_kni *kni, struct rte_mbuf **mbufs, unsigned int num)
 {
-	unsigned ret = kni_fifo_get(kni->tx_q, (void **)mbufs, num);
+	unsigned int ret = kni_fifo_get(kni->tx_q, (void **)mbufs, num);
 
 	/* If buffers removed, allocate mbufs and then put them into alloc_q */
 	if (ret)
@@ -605,7 +605,7 @@ kni_allocate_mbufs(struct rte_kni *kni)
 		return;
 	}
 
-	allocq_free = (kni->alloc_q->read - kni->alloc_q->write - 1) \
+	allocq_free = (kni->alloc_q->read - kni->alloc_q->write - 1)
 			& (MAX_MBUF_BURST_NUM - 1);
 	for (i = 0; i < allocq_free; i++) {
 		pkts[i] = rte_pktmbuf_alloc(kni->pktmbuf_pool);
@@ -659,35 +659,35 @@ static enum kni_ops_status
 kni_check_request_register(struct rte_kni_ops *ops)
 {
 	/* check if KNI request ops has been registered*/
-	if( NULL == ops )
+	if (ops == NULL)
 		return KNI_REQ_NO_REGISTER;
 
-	if ((ops->change_mtu == NULL)
-		&& (ops->config_network_if == NULL)
-		&& (ops->config_mac_address == NULL)
-		&& (ops->config_promiscusity == NULL))
+	if (ops->change_mtu == NULL
+	    && ops->config_network_if == NULL
+	    && ops->config_mac_address == NULL
+	    && ops->config_promiscusity == NULL)
 		return KNI_REQ_NO_REGISTER;
 
 	return KNI_REQ_REGISTERED;
 }
 
 int
-rte_kni_register_handlers(struct rte_kni *kni,struct rte_kni_ops *ops)
+rte_kni_register_handlers(struct rte_kni *kni, struct rte_kni_ops *ops)
 {
 	enum kni_ops_status req_status;
 
-	if (NULL == ops) {
+	if (ops == NULL) {
 		RTE_LOG(ERR, KNI, "Invalid KNI request operation.\n");
 		return -1;
 	}
 
-	if (NULL == kni) {
+	if (kni == NULL) {
 		RTE_LOG(ERR, KNI, "Invalid kni info.\n");
 		return -1;
 	}
 
 	req_status = kni_check_request_register(&kni->ops);
-	if ( KNI_REQ_REGISTERED == req_status) {
+	if (req_status == KNI_REQ_REGISTERED) {
 		RTE_LOG(ERR, KNI, "The KNI request operation has already registered.\n");
 		return -1;
 	}
@@ -699,7 +699,7 @@ rte_kni_register_handlers(struct rte_kni *kni,struct rte_kni_ops *ops)
 int
 rte_kni_unregister_handlers(struct rte_kni *kni)
 {
-	if (NULL == kni) {
+	if (kni == NULL) {
 		RTE_LOG(ERR, KNI, "Invalid kni info.\n");
 		return -1;
 	}
-- 
2.20.1


^ permalink raw reply	[flat|nested] 63+ messages in thread

* [dpdk-dev] [PATCH v5 9/9] kni: add minimal ethtool
  2019-06-20 19:20 ` [dpdk-dev] [PATCH v5 0/9] kni: fixes and cleanups Stephen Hemminger
                     ` (7 preceding siblings ...)
  2019-06-20 19:20   ` [dpdk-dev] [PATCH v5 8/9] kni: fix style issues Stephen Hemminger
@ 2019-06-20 19:20   ` Stephen Hemminger
  2019-06-21  8:26     ` Igor Ryzhov
  8 siblings, 1 reply; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-20 19:20 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

Some applications use ethtool so add the minimum ethtool ops.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 kernel/linux/kni/kni_dev.h  |  2 ++
 kernel/linux/kni/kni_misc.c |  1 +
 kernel/linux/kni/kni_net.c  | 14 ++++++++++++++
 3 files changed, 17 insertions(+)

diff --git a/kernel/linux/kni/kni_dev.h b/kernel/linux/kni/kni_dev.h
index ceba5f73c1d9..c1ca6789ce12 100644
--- a/kernel/linux/kni/kni_dev.h
+++ b/kernel/linux/kni/kni_dev.h
@@ -11,6 +11,8 @@
 #endif
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
+#define KNI_VERSION	"1.0"
+
 #include "compat.h"
 
 #include <linux/if.h>
diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c
index be45f823408f..2b75502a8b0e 100644
--- a/kernel/linux/kni/kni_misc.c
+++ b/kernel/linux/kni/kni_misc.c
@@ -21,6 +21,7 @@
 #include "compat.h"
 #include "kni_dev.h"
 
+MODULE_VERSION(KNI_VERSION);
 MODULE_LICENSE("Dual BSD/GPL");
 MODULE_AUTHOR("Intel Corporation");
 MODULE_DESCRIPTION("Kernel Module for managing kni devices");
diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c
index 320d51d7fc83..319ee2dcb19a 100644
--- a/kernel/linux/kni/kni_net.c
+++ b/kernel/linux/kni/kni_net.c
@@ -13,6 +13,7 @@
 #include <linux/version.h>
 #include <linux/netdevice.h>
 #include <linux/etherdevice.h> /* eth_type_trans */
+#include <linux/ethtool.h>
 #include <linux/skbuff.h>
 #include <linux/kthread.h>
 #include <linux/delay.h>
@@ -725,6 +726,18 @@ static const struct net_device_ops kni_net_netdev_ops = {
 #endif
 };
 
+static void kni_get_drvinfo(struct net_device *dev,
+			    struct ethtool_drvinfo *info)
+{
+	strlcpy(info->version, KNI_VERSION, sizeof(info->version));
+	strlcpy(info->driver, "kni", sizeof(info->driver));
+}
+
+static const struct ethtool_ops kni_net_ethtool_ops = {
+	.get_drvinfo	= kni_get_drvinfo,
+	.get_link	= ethtool_op_get_link,
+};
+
 void
 kni_net_init(struct net_device *dev)
 {
@@ -736,6 +749,7 @@ kni_net_init(struct net_device *dev)
 	ether_setup(dev); /* assign some of the fields */
 	dev->netdev_ops      = &kni_net_netdev_ops;
 	dev->header_ops      = &kni_net_header_ops;
+	dev->ethtool_ops     = &kni_net_ethtool_ops;
 	dev->watchdog_timeo = WD_TIMEOUT;
 }
 
-- 
2.20.1


^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [dpdk-dev] [PATCH v5 9/9] kni: add minimal ethtool
  2019-06-20 19:20   ` [dpdk-dev] [PATCH v5 9/9] kni: add minimal ethtool Stephen Hemminger
@ 2019-06-21  8:26     ` Igor Ryzhov
  2019-06-21 15:10       ` Stephen Hemminger
  0 siblings, 1 reply; 63+ messages in thread
From: Igor Ryzhov @ 2019-06-21  8:26 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev

Hi Stephen,

As ethtool support is restored, I think your patch #7 should be
updated to keep ethtool support in docs.

Best regards,
Igor

On Thu, Jun 20, 2019 at 10:22 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> Some applications use ethtool so add the minimum ethtool ops.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  kernel/linux/kni/kni_dev.h  |  2 ++
>  kernel/linux/kni/kni_misc.c |  1 +
>  kernel/linux/kni/kni_net.c  | 14 ++++++++++++++
>  3 files changed, 17 insertions(+)
>
> diff --git a/kernel/linux/kni/kni_dev.h b/kernel/linux/kni/kni_dev.h
> index ceba5f73c1d9..c1ca6789ce12 100644
> --- a/kernel/linux/kni/kni_dev.h
> +++ b/kernel/linux/kni/kni_dev.h
> @@ -11,6 +11,8 @@
>  #endif
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>
> +#define KNI_VERSION    "1.0"
> +
>  #include "compat.h"
>
>  #include <linux/if.h>
> diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c
> index be45f823408f..2b75502a8b0e 100644
> --- a/kernel/linux/kni/kni_misc.c
> +++ b/kernel/linux/kni/kni_misc.c
> @@ -21,6 +21,7 @@
>  #include "compat.h"
>  #include "kni_dev.h"
>
> +MODULE_VERSION(KNI_VERSION);
>  MODULE_LICENSE("Dual BSD/GPL");
>  MODULE_AUTHOR("Intel Corporation");
>  MODULE_DESCRIPTION("Kernel Module for managing kni devices");
> diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c
> index 320d51d7fc83..319ee2dcb19a 100644
> --- a/kernel/linux/kni/kni_net.c
> +++ b/kernel/linux/kni/kni_net.c
> @@ -13,6 +13,7 @@
>  #include <linux/version.h>
>  #include <linux/netdevice.h>
>  #include <linux/etherdevice.h> /* eth_type_trans */
> +#include <linux/ethtool.h>
>  #include <linux/skbuff.h>
>  #include <linux/kthread.h>
>  #include <linux/delay.h>
> @@ -725,6 +726,18 @@ static const struct net_device_ops kni_net_netdev_ops = {
>  #endif
>  };
>
> +static void kni_get_drvinfo(struct net_device *dev,
> +                           struct ethtool_drvinfo *info)
> +{
> +       strlcpy(info->version, KNI_VERSION, sizeof(info->version));
> +       strlcpy(info->driver, "kni", sizeof(info->driver));
> +}
> +
> +static const struct ethtool_ops kni_net_ethtool_ops = {
> +       .get_drvinfo    = kni_get_drvinfo,
> +       .get_link       = ethtool_op_get_link,
> +};
> +
>  void
>  kni_net_init(struct net_device *dev)
>  {
> @@ -736,6 +749,7 @@ kni_net_init(struct net_device *dev)
>         ether_setup(dev); /* assign some of the fields */
>         dev->netdev_ops      = &kni_net_netdev_ops;
>         dev->header_ops      = &kni_net_header_ops;
> +       dev->ethtool_ops     = &kni_net_ethtool_ops;
>         dev->watchdog_timeo = WD_TIMEOUT;
>  }
>
> --
> 2.20.1
>

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [dpdk-dev] [PATCH v5 9/9] kni: add minimal ethtool
  2019-06-21  8:26     ` Igor Ryzhov
@ 2019-06-21 15:10       ` Stephen Hemminger
  0 siblings, 0 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-21 15:10 UTC (permalink / raw)
  To: Igor Ryzhov; +Cc: dev

On Fri, 21 Jun 2019 11:26:09 +0300
Igor Ryzhov <iryzhov@nfware.com> wrote:

> Hi Stephen,
> 
> As ethtool support is restored, I think your patch #7 should be
> updated to keep ethtool support in docs.
> 
> Best regards,
> Igor

Good idea, I will add that. This is not like the original ethtool,
it is intentionally limited in scope and generic (unlike the original).


^ permalink raw reply	[flat|nested] 63+ messages in thread

* [dpdk-dev] [PATCH v6 0/9] kni: fixes and cleanups
  2019-06-10 17:51 [dpdk-dev] [PATCH v2 0/7] kni: cleanups and improvements Stephen Hemminger
                   ` (11 preceding siblings ...)
  2019-06-20 19:20 ` [dpdk-dev] [PATCH v5 0/9] kni: fixes and cleanups Stephen Hemminger
@ 2019-06-24 16:47 ` Stephen Hemminger
  2019-06-24 16:47   ` [dpdk-dev] [PATCH v6 1/9] kni: don't need stubs for rx_mode or ioctl Stephen Hemminger
                     ` (9 more replies)
  12 siblings, 10 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-24 16:47 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

While testing KNI with netvsc, saw lots of places more code
could be safely removed from KNI kernel driver.

v6 - more updates to documentation
v5 - add minimal ethtool, fix checkpath author complaints
v4 - add more style fixes
v3 - rebase to current master, add style fix patch
v2 - get rid of unnecessary padding, combine the unused field patches

Stephen Hemminger (9):
  kni: don't need stubs for rx_mode or ioctl
  kni: use netdev_alloc_skb
  kni: don't keep stats in kni_net
  kni: drop unused fields
  kni: use proper type for kni fifo's
  kni: return -EFAULT if copy_from_user fails
  kni: fix style issues
  kni: add minimal ethtool
  doc: update KNI documentation

 .../prog_guide/kernel_nic_interface.rst       |   9 +-
 .../sample_app_ug/kernel_nic_interface.rst    |  24 +++--
 kernel/linux/kni/kni_dev.h                    |  20 ++--
 kernel/linux/kni/kni_misc.c                   |  18 ++--
 kernel/linux/kni/kni_net.c                    | 100 +++++++-----------
 lib/librte_kni/rte_kni.c                      |  38 +++----
 6 files changed, 89 insertions(+), 120 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 63+ messages in thread

* [dpdk-dev] [PATCH v6 1/9] kni: don't need stubs for rx_mode or ioctl
  2019-06-24 16:47 ` [dpdk-dev] [PATCH v6 0/9] kni: fixes and cleanups Stephen Hemminger
@ 2019-06-24 16:47   ` Stephen Hemminger
  2019-06-24 16:47   ` [dpdk-dev] [PATCH v6 2/9] kni: use netdev_alloc_skb Stephen Hemminger
                     ` (8 subsequent siblings)
  9 siblings, 0 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-24 16:47 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

The netdev subsystem already handles case where
network sevice does not support ioctl.

If device has no rx_mode hook it is not called.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 kernel/linux/kni/kni_net.c | 19 -------------------
 1 file changed, 19 deletions(-)

diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c
index ad8365877cda..c86337d099ab 100644
--- a/kernel/linux/kni/kni_net.c
+++ b/kernel/linux/kni/kni_net.c
@@ -593,23 +593,6 @@ kni_net_tx_timeout(struct net_device *dev)
 	netif_wake_queue(dev);
 }
 
-/*
- * Ioctl commands
- */
-static int
-kni_net_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
-{
-	pr_debug("kni_net_ioctl group:%d cmd:%d\n",
-		((struct kni_dev *)netdev_priv(dev))->group_id, cmd);
-
-	return -EOPNOTSUPP;
-}
-
-static void
-kni_net_set_rx_mode(struct net_device *dev)
-{
-}
-
 static int
 kni_net_change_mtu(struct net_device *dev, int new_mtu)
 {
@@ -758,8 +741,6 @@ static const struct net_device_ops kni_net_netdev_ops = {
 	.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,
-	.ndo_set_rx_mode = kni_net_set_rx_mode,
 	.ndo_get_stats = kni_net_stats,
 	.ndo_tx_timeout = kni_net_tx_timeout,
 	.ndo_set_mac_address = kni_net_set_mac,
-- 
2.20.1


^ permalink raw reply	[flat|nested] 63+ messages in thread

* [dpdk-dev] [PATCH v6 2/9] kni: use netdev_alloc_skb
  2019-06-24 16:47 ` [dpdk-dev] [PATCH v6 0/9] kni: fixes and cleanups Stephen Hemminger
  2019-06-24 16:47   ` [dpdk-dev] [PATCH v6 1/9] kni: don't need stubs for rx_mode or ioctl Stephen Hemminger
@ 2019-06-24 16:47   ` Stephen Hemminger
  2019-06-24 16:47   ` [dpdk-dev] [PATCH v6 3/9] kni: don't keep stats in kni_net Stephen Hemminger
                     ` (7 subsequent siblings)
  9 siblings, 0 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-24 16:47 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

netdev_alloc_skb is optimized to any alignment or setup
of skb->dev that is required. The kernel has chosen to not pad
packets on x86 (for many years), because it is faster.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 kernel/linux/kni/kni_net.c | 17 +++--------------
 1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c
index c86337d099ab..cce5e7eb991f 100644
--- a/kernel/linux/kni/kni_net.c
+++ b/kernel/linux/kni/kni_net.c
@@ -340,16 +340,13 @@ kni_net_rx_normal(struct kni_dev *kni)
 		data_kva = kva2data_kva(kva);
 		kni->va[i] = pa2va(kni->pa[i], kva);
 
-		skb = dev_alloc_skb(len + 2);
+		skb = netdev_alloc_skb(dev, len);
 		if (!skb) {
 			/* Update statistics */
 			kni->stats.rx_dropped++;
 			continue;
 		}
 
-		/* Align IP on 16B boundary */
-		skb_reserve(skb, 2);
-
 		if (kva->nb_segs == 1) {
 			memcpy(skb_put(skb, len), data_kva, len);
 		} else {
@@ -368,7 +365,6 @@ kni_net_rx_normal(struct kni_dev *kni)
 			}
 		}
 
-		skb->dev = dev;
 		skb->protocol = eth_type_trans(skb, dev);
 		skb->ip_summed = CHECKSUM_UNNECESSARY;
 
@@ -512,26 +508,20 @@ kni_net_rx_lo_fifo_skb(struct kni_dev *kni)
 		data_kva = kva2data_kva(kva);
 		kni->va[i] = pa2va(kni->pa[i], kva);
 
-		skb = dev_alloc_skb(len + 2);
+		skb = netdev_alloc_skb(dev, len);
 		if (skb) {
-			/* Align IP on 16B boundary */
-			skb_reserve(skb, 2);
 			memcpy(skb_put(skb, len), data_kva, len);
-			skb->dev = dev;
 			skb->ip_summed = CHECKSUM_UNNECESSARY;
 			dev_kfree_skb(skb);
 		}
 
 		/* Simulate real usage, allocate/copy skb twice */
-		skb = dev_alloc_skb(len + 2);
+		skb = netdev_alloc_skb(dev, len);
 		if (skb == NULL) {
 			kni->stats.rx_dropped++;
 			continue;
 		}
 
-		/* Align IP on 16B boundary */
-		skb_reserve(skb, 2);
-
 		if (kva->nb_segs == 1) {
 			memcpy(skb_put(skb, len), data_kva, len);
 		} else {
@@ -550,7 +540,6 @@ kni_net_rx_lo_fifo_skb(struct kni_dev *kni)
 			}
 		}
 
-		skb->dev = dev;
 		skb->ip_summed = CHECKSUM_UNNECESSARY;
 
 		kni->stats.rx_bytes += len;
-- 
2.20.1


^ permalink raw reply	[flat|nested] 63+ messages in thread

* [dpdk-dev] [PATCH v6 3/9] kni: don't keep stats in kni_net
  2019-06-24 16:47 ` [dpdk-dev] [PATCH v6 0/9] kni: fixes and cleanups Stephen Hemminger
  2019-06-24 16:47   ` [dpdk-dev] [PATCH v6 1/9] kni: don't need stubs for rx_mode or ioctl Stephen Hemminger
  2019-06-24 16:47   ` [dpdk-dev] [PATCH v6 2/9] kni: use netdev_alloc_skb Stephen Hemminger
@ 2019-06-24 16:47   ` Stephen Hemminger
  2019-06-24 16:47   ` [dpdk-dev] [PATCH v6 4/9] kni: drop unused fields Stephen Hemminger
                     ` (6 subsequent siblings)
  9 siblings, 0 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-24 16:47 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

Since kernel 2.6.28 the network subsystem has provided
dev->stats for devices to use statistics handling and is the
default if no ndo_get_stats is provided.

This allow allows for 64 bit (rather than just 32 bit)
statistics with KNI.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 kernel/linux/kni/kni_dev.h |  1 -
 kernel/linux/kni/kni_net.c | 43 +++++++++++++-------------------------
 2 files changed, 15 insertions(+), 29 deletions(-)

diff --git a/kernel/linux/kni/kni_dev.h b/kernel/linux/kni/kni_dev.h
index d57bce647e4a..21e4b0d92368 100644
--- a/kernel/linux/kni/kni_dev.h
+++ b/kernel/linux/kni/kni_dev.h
@@ -39,7 +39,6 @@ struct kni_dev {
 	/* kni list */
 	struct list_head list;
 
-	struct net_device_stats stats;
 	int status;
 	uint16_t group_id;           /* Group ID of a group of KNI devices */
 	uint32_t core_id;            /* Core ID to bind */
diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c
index cce5e7eb991f..06310fec57bb 100644
--- a/kernel/linux/kni/kni_net.c
+++ b/kernel/linux/kni/kni_net.c
@@ -291,15 +291,15 @@ kni_net_tx(struct sk_buff *skb, struct net_device *dev)
 
 	/* Free skb and update statistics */
 	dev_kfree_skb(skb);
-	kni->stats.tx_bytes += len;
-	kni->stats.tx_packets++;
+	dev->stats.tx_bytes += len;
+	dev->stats.tx_packets++;
 
 	return NETDEV_TX_OK;
 
 drop:
 	/* Free skb and update statistics */
 	dev_kfree_skb(skb);
-	kni->stats.tx_dropped++;
+	dev->stats.tx_dropped++;
 
 	return NETDEV_TX_OK;
 }
@@ -343,7 +343,7 @@ kni_net_rx_normal(struct kni_dev *kni)
 		skb = netdev_alloc_skb(dev, len);
 		if (!skb) {
 			/* Update statistics */
-			kni->stats.rx_dropped++;
+			dev->stats.rx_dropped++;
 			continue;
 		}
 
@@ -372,8 +372,8 @@ kni_net_rx_normal(struct kni_dev *kni)
 		netif_rx_ni(skb);
 
 		/* Update statistics */
-		kni->stats.rx_bytes += len;
-		kni->stats.rx_packets++;
+		dev->stats.rx_bytes += len;
+		dev->stats.rx_packets++;
 	}
 
 	/* Burst enqueue mbufs into free_q */
@@ -396,6 +396,7 @@ kni_net_rx_lo_fifo(struct kni_dev *kni)
 	void *data_kva;
 	struct rte_kni_mbuf *alloc_kva;
 	void *alloc_data_kva;
+	struct net_device *dev = kni->net_dev;
 
 	/* Get the number of entries in rx_q */
 	num_rq = kni_fifo_count(kni->rx_q);
@@ -443,8 +444,8 @@ kni_net_rx_lo_fifo(struct kni_dev *kni)
 			alloc_kva->pkt_len = len;
 			alloc_kva->data_len = len;
 
-			kni->stats.tx_bytes += len;
-			kni->stats.rx_bytes += len;
+			dev->stats.tx_bytes += len;
+			dev->stats.rx_bytes += len;
 		}
 
 		/* Burst enqueue mbufs into tx_q */
@@ -464,8 +465,8 @@ kni_net_rx_lo_fifo(struct kni_dev *kni)
 	 * Update statistic, and enqueue/dequeue failure is impossible,
 	 * as all queues are checked at first.
 	 */
-	kni->stats.tx_packets += num;
-	kni->stats.rx_packets += num;
+	dev->stats.tx_packets += num;
+	dev->stats.rx_packets += num;
 }
 
 /*
@@ -518,7 +519,7 @@ kni_net_rx_lo_fifo_skb(struct kni_dev *kni)
 		/* Simulate real usage, allocate/copy skb twice */
 		skb = netdev_alloc_skb(dev, len);
 		if (skb == NULL) {
-			kni->stats.rx_dropped++;
+			dev->stats.rx_dropped++;
 			continue;
 		}
 
@@ -542,8 +543,8 @@ kni_net_rx_lo_fifo_skb(struct kni_dev *kni)
 
 		skb->ip_summed = CHECKSUM_UNNECESSARY;
 
-		kni->stats.rx_bytes += len;
-		kni->stats.rx_packets++;
+		dev->stats.rx_bytes += len;
+		dev->stats.rx_packets++;
 
 		/* call tx interface */
 		kni_net_tx(skb, dev);
@@ -573,12 +574,10 @@ kni_net_rx(struct kni_dev *kni)
 static void
 kni_net_tx_timeout(struct net_device *dev)
 {
-	struct kni_dev *kni = netdev_priv(dev);
-
 	pr_debug("Transmit timeout at %ld, latency %ld\n", jiffies,
 			jiffies - dev_trans_start(dev));
 
-	kni->stats.tx_errors++;
+	dev->stats.tx_errors++;
 	netif_wake_queue(dev);
 }
 
@@ -627,17 +626,6 @@ kni_net_poll_resp(struct kni_dev *kni)
 		wake_up_interruptible(&kni->wq);
 }
 
-/*
- * Return statistics to the caller
- */
-static struct net_device_stats *
-kni_net_stats(struct net_device *dev)
-{
-	struct kni_dev *kni = netdev_priv(dev);
-
-	return &kni->stats;
-}
-
 /*
  *  Fill the eth header
  */
@@ -730,7 +718,6 @@ static const struct net_device_ops kni_net_netdev_ops = {
 	.ndo_change_rx_flags = kni_net_set_promiscusity,
 	.ndo_start_xmit = kni_net_tx,
 	.ndo_change_mtu = kni_net_change_mtu,
-	.ndo_get_stats = kni_net_stats,
 	.ndo_tx_timeout = kni_net_tx_timeout,
 	.ndo_set_mac_address = kni_net_set_mac,
 #ifdef HAVE_CHANGE_CARRIER_CB
-- 
2.20.1


^ permalink raw reply	[flat|nested] 63+ messages in thread

* [dpdk-dev] [PATCH v6 4/9] kni: drop unused fields
  2019-06-24 16:47 ` [dpdk-dev] [PATCH v6 0/9] kni: fixes and cleanups Stephen Hemminger
                     ` (2 preceding siblings ...)
  2019-06-24 16:47   ` [dpdk-dev] [PATCH v6 3/9] kni: don't keep stats in kni_net Stephen Hemminger
@ 2019-06-24 16:47   ` Stephen Hemminger
  2019-06-24 16:47   ` [dpdk-dev] [PATCH v6 5/9] kni: use proper type for kni fifo's Stephen Hemminger
                     ` (5 subsequent siblings)
  9 siblings, 0 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-24 16:47 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

Several fields were either totally unused or set and never used.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 kernel/linux/kni/kni_dev.h  | 5 -----
 kernel/linux/kni/kni_misc.c | 1 -
 2 files changed, 6 deletions(-)

diff --git a/kernel/linux/kni/kni_dev.h b/kernel/linux/kni/kni_dev.h
index 21e4b0d92368..f3e6c3ca4efa 100644
--- a/kernel/linux/kni/kni_dev.h
+++ b/kernel/linux/kni/kni_dev.h
@@ -39,8 +39,6 @@ struct kni_dev {
 	/* kni list */
 	struct list_head list;
 
-	int status;
-	uint16_t group_id;           /* Group ID of a group of KNI devices */
 	uint32_t core_id;            /* Core ID to bind */
 	char name[RTE_KNI_NAMESIZE]; /* Network device name */
 	struct task_struct *pthread;
@@ -79,9 +77,6 @@ struct kni_dev {
 	/* mbuf size */
 	uint32_t mbuf_size;
 
-	/* synchro for request processing */
-	unsigned long synchro;
-
 	/* buffers */
 	void *pa[MBUF_BURST_SZ];
 	void *va[MBUF_BURST_SZ];
diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c
index 1fc5eeb9c8ca..b59cf24c2184 100644
--- a/kernel/linux/kni/kni_misc.c
+++ b/kernel/linux/kni/kni_misc.c
@@ -346,7 +346,6 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num,
 	kni = netdev_priv(net_dev);
 
 	kni->net_dev = net_dev;
-	kni->group_id = dev_info.group_id;
 	kni->core_id = dev_info.core_id;
 	strncpy(kni->name, dev_info.name, RTE_KNI_NAMESIZE);
 
-- 
2.20.1


^ permalink raw reply	[flat|nested] 63+ messages in thread

* [dpdk-dev] [PATCH v6 5/9] kni: use proper type for kni fifo's
  2019-06-24 16:47 ` [dpdk-dev] [PATCH v6 0/9] kni: fixes and cleanups Stephen Hemminger
                     ` (3 preceding siblings ...)
  2019-06-24 16:47   ` [dpdk-dev] [PATCH v6 4/9] kni: drop unused fields Stephen Hemminger
@ 2019-06-24 16:47   ` Stephen Hemminger
  2019-06-24 16:47   ` [dpdk-dev] [PATCH v6 6/9] kni: return -EFAULT if copy_from_user fails Stephen Hemminger
                     ` (4 subsequent siblings)
  9 siblings, 0 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-24 16:47 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

Using void * instead of proper type is unsafe practice.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 kernel/linux/kni/kni_dev.h | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/kernel/linux/kni/kni_dev.h b/kernel/linux/kni/kni_dev.h
index f3e6c3ca4efa..ceba5f73c1d9 100644
--- a/kernel/linux/kni/kni_dev.h
+++ b/kernel/linux/kni/kni_dev.h
@@ -51,22 +51,22 @@ struct kni_dev {
 	struct net_device *net_dev;
 
 	/* queue for packets to be sent out */
-	void *tx_q;
+	struct rte_kni_fifo *tx_q;
 
 	/* queue for the packets received */
-	void *rx_q;
+	struct rte_kni_fifo *rx_q;
 
 	/* queue for the allocated mbufs those can be used to save sk buffs */
-	void *alloc_q;
+	struct rte_kni_fifo *alloc_q;
 
 	/* free queue for the mbufs to be freed */
-	void *free_q;
+	struct rte_kni_fifo *free_q;
 
 	/* request queue */
-	void *req_q;
+	struct rte_kni_fifo *req_q;
 
 	/* response queue */
-	void *resp_q;
+	struct rte_kni_fifo *resp_q;
 
 	void *sync_kva;
 	void *sync_va;
-- 
2.20.1


^ permalink raw reply	[flat|nested] 63+ messages in thread

* [dpdk-dev] [PATCH v6 6/9] kni: return -EFAULT if copy_from_user fails
  2019-06-24 16:47 ` [dpdk-dev] [PATCH v6 0/9] kni: fixes and cleanups Stephen Hemminger
                     ` (4 preceding siblings ...)
  2019-06-24 16:47   ` [dpdk-dev] [PATCH v6 5/9] kni: use proper type for kni fifo's Stephen Hemminger
@ 2019-06-24 16:47   ` Stephen Hemminger
  2019-06-24 16:47   ` [dpdk-dev] [PATCH v6 7/9] kni: fix style issues Stephen Hemminger
                     ` (3 subsequent siblings)
  9 siblings, 0 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-24 16:47 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

The correct thing to return if user gives a bad data
is to return -EFAULT. Logging is also discouraged because
it could be used as a DoS attack.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 kernel/linux/kni/kni_misc.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c
index b59cf24c2184..be45f823408f 100644
--- a/kernel/linux/kni/kni_misc.c
+++ b/kernel/linux/kni/kni_misc.c
@@ -301,11 +301,8 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num,
 		return -EINVAL;
 
 	/* Copy kni info from user space */
-	ret = copy_from_user(&dev_info, (void *)ioctl_param, sizeof(dev_info));
-	if (ret) {
-		pr_err("copy_from_user in kni_ioctl_create");
-		return -EIO;
-	}
+	if (copy_from_user(&dev_info, (void *)ioctl_param, sizeof(dev_info)))
+		return -EFAULT;
 
 	/* Check if name is zero-ended */
 	if (strnlen(dev_info.name, sizeof(dev_info.name)) == sizeof(dev_info.name)) {
@@ -427,15 +424,12 @@ kni_ioctl_release(struct net *net, uint32_t ioctl_num,
 	if (_IOC_SIZE(ioctl_num) > sizeof(dev_info))
 		return -EINVAL;
 
-	ret = copy_from_user(&dev_info, (void *)ioctl_param, sizeof(dev_info));
-	if (ret) {
-		pr_err("copy_from_user in kni_ioctl_release");
-		return -EIO;
-	}
+	if (copy_from_user(&dev_info, (void *)ioctl_param, sizeof(dev_info)))
+		return -EFAULT;
 
 	/* Release the network device according to its name */
 	if (strlen(dev_info.name) == 0)
-		return ret;
+		return -EINVAL;
 
 	down_write(&knet->kni_list_lock);
 	list_for_each_entry_safe(dev, n, &knet->kni_list_head, list) {
-- 
2.20.1


^ permalink raw reply	[flat|nested] 63+ messages in thread

* [dpdk-dev] [PATCH v6 7/9] kni: fix style issues
  2019-06-24 16:47 ` [dpdk-dev] [PATCH v6 0/9] kni: fixes and cleanups Stephen Hemminger
                     ` (5 preceding siblings ...)
  2019-06-24 16:47   ` [dpdk-dev] [PATCH v6 6/9] kni: return -EFAULT if copy_from_user fails Stephen Hemminger
@ 2019-06-24 16:47   ` Stephen Hemminger
  2019-06-24 16:47   ` [dpdk-dev] [PATCH v6 8/9] kni: add minimal ethtool Stephen Hemminger
                     ` (2 subsequent siblings)
  9 siblings, 0 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-24 16:47 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

rte_kni does not follow standard style rules.
Noticed some extra \ line continuation etc.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 kernel/linux/kni/kni_net.c |  7 ++++---
 lib/librte_kni/rte_kni.c   | 38 +++++++++++++++++++-------------------
 2 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c
index 06310fec57bb..320d51d7fc83 100644
--- a/kernel/linux/kni/kni_net.c
+++ b/kernel/linux/kni/kni_net.c
@@ -401,7 +401,7 @@ kni_net_rx_lo_fifo(struct kni_dev *kni)
 	/* Get the number of entries in rx_q */
 	num_rq = kni_fifo_count(kni->rx_q);
 
-	/* Get the number of free entrie in tx_q */
+	/* Get the number of free entries in tx_q */
 	num_tq = kni_fifo_free_count(kni->tx_q);
 
 	/* Get the number of entries in alloc_q */
@@ -755,6 +755,7 @@ kni_net_config_lo_mode(char *lo_str)
 	} else if (!strcmp(lo_str, "lo_mode_fifo_skb")) {
 		pr_debug("loopback mode=lo_mode_fifo_skb enabled");
 		kni_net_rx_func = kni_net_rx_lo_fifo_skb;
-	} else
-		pr_debug("Incognizant parameter, loopback disabled");
+	} else {
+		pr_debug("Unknown loopback parameter, disabled");
+	}
 }
diff --git a/lib/librte_kni/rte_kni.c b/lib/librte_kni/rte_kni.c
index e29d0cc7df3c..9b6acc382fc3 100644
--- a/lib/librte_kni/rte_kni.c
+++ b/lib/librte_kni/rte_kni.c
@@ -59,7 +59,7 @@ struct rte_kni {
 	uint16_t group_id;                  /**< Group ID of KNI devices */
 	uint32_t slot_id;                   /**< KNI pool slot ID */
 	struct rte_mempool *pktmbuf_pool;   /**< pkt mbuf mempool */
-	unsigned mbuf_size;                 /**< mbuf size */
+	unsigned int mbuf_size;                 /**< mbuf size */
 
 	const struct rte_memzone *m_tx_q;   /**< TX queue memzone */
 	const struct rte_memzone *m_rx_q;   /**< RX queue memzone */
@@ -78,7 +78,7 @@ struct rte_kni {
 	/* For request & response */
 	struct rte_kni_fifo *req_q;         /**< Request queue */
 	struct rte_kni_fifo *resp_q;        /**< Response queue */
-	void * sync_addr;                   /**< Req/Resp Mem address */
+	void *sync_addr;                   /**< Req/Resp Mem address */
 
 	struct rte_kni_ops ops;             /**< operations for request */
 };
@@ -473,7 +473,7 @@ kni_config_promiscusity(uint16_t port_id, uint8_t to_on)
 int
 rte_kni_handle_request(struct rte_kni *kni)
 {
-	unsigned ret;
+	unsigned int ret;
 	struct rte_kni_request *req = NULL;
 
 	if (kni == NULL)
@@ -498,8 +498,8 @@ rte_kni_handle_request(struct rte_kni *kni)
 		break;
 	case RTE_KNI_REQ_CFG_NETWORK_IF: /* Set network interface up/down */
 		if (kni->ops.config_network_if)
-			req->result = kni->ops.config_network_if(\
-					kni->ops.port_id, req->if_up);
+			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)
@@ -534,7 +534,7 @@ rte_kni_handle_request(struct rte_kni *kni)
 }
 
 unsigned
-rte_kni_tx_burst(struct rte_kni *kni, struct rte_mbuf **mbufs, unsigned num)
+rte_kni_tx_burst(struct rte_kni *kni, struct rte_mbuf **mbufs, unsigned int num)
 {
 	void *phy_mbufs[num];
 	unsigned int ret;
@@ -552,9 +552,9 @@ rte_kni_tx_burst(struct rte_kni *kni, struct rte_mbuf **mbufs, unsigned num)
 }
 
 unsigned
-rte_kni_rx_burst(struct rte_kni *kni, struct rte_mbuf **mbufs, unsigned num)
+rte_kni_rx_burst(struct rte_kni *kni, struct rte_mbuf **mbufs, unsigned int num)
 {
-	unsigned ret = kni_fifo_get(kni->tx_q, (void **)mbufs, num);
+	unsigned int ret = kni_fifo_get(kni->tx_q, (void **)mbufs, num);
 
 	/* If buffers removed, allocate mbufs and then put them into alloc_q */
 	if (ret)
@@ -605,7 +605,7 @@ kni_allocate_mbufs(struct rte_kni *kni)
 		return;
 	}
 
-	allocq_free = (kni->alloc_q->read - kni->alloc_q->write - 1) \
+	allocq_free = (kni->alloc_q->read - kni->alloc_q->write - 1)
 			& (MAX_MBUF_BURST_NUM - 1);
 	for (i = 0; i < allocq_free; i++) {
 		pkts[i] = rte_pktmbuf_alloc(kni->pktmbuf_pool);
@@ -659,35 +659,35 @@ static enum kni_ops_status
 kni_check_request_register(struct rte_kni_ops *ops)
 {
 	/* check if KNI request ops has been registered*/
-	if( NULL == ops )
+	if (ops == NULL)
 		return KNI_REQ_NO_REGISTER;
 
-	if ((ops->change_mtu == NULL)
-		&& (ops->config_network_if == NULL)
-		&& (ops->config_mac_address == NULL)
-		&& (ops->config_promiscusity == NULL))
+	if (ops->change_mtu == NULL
+	    && ops->config_network_if == NULL
+	    && ops->config_mac_address == NULL
+	    && ops->config_promiscusity == NULL)
 		return KNI_REQ_NO_REGISTER;
 
 	return KNI_REQ_REGISTERED;
 }
 
 int
-rte_kni_register_handlers(struct rte_kni *kni,struct rte_kni_ops *ops)
+rte_kni_register_handlers(struct rte_kni *kni, struct rte_kni_ops *ops)
 {
 	enum kni_ops_status req_status;
 
-	if (NULL == ops) {
+	if (ops == NULL) {
 		RTE_LOG(ERR, KNI, "Invalid KNI request operation.\n");
 		return -1;
 	}
 
-	if (NULL == kni) {
+	if (kni == NULL) {
 		RTE_LOG(ERR, KNI, "Invalid kni info.\n");
 		return -1;
 	}
 
 	req_status = kni_check_request_register(&kni->ops);
-	if ( KNI_REQ_REGISTERED == req_status) {
+	if (req_status == KNI_REQ_REGISTERED) {
 		RTE_LOG(ERR, KNI, "The KNI request operation has already registered.\n");
 		return -1;
 	}
@@ -699,7 +699,7 @@ rte_kni_register_handlers(struct rte_kni *kni,struct rte_kni_ops *ops)
 int
 rte_kni_unregister_handlers(struct rte_kni *kni)
 {
-	if (NULL == kni) {
+	if (kni == NULL) {
 		RTE_LOG(ERR, KNI, "Invalid kni info.\n");
 		return -1;
 	}
-- 
2.20.1


^ permalink raw reply	[flat|nested] 63+ messages in thread

* [dpdk-dev] [PATCH v6 8/9] kni: add minimal ethtool
  2019-06-24 16:47 ` [dpdk-dev] [PATCH v6 0/9] kni: fixes and cleanups Stephen Hemminger
                     ` (6 preceding siblings ...)
  2019-06-24 16:47   ` [dpdk-dev] [PATCH v6 7/9] kni: fix style issues Stephen Hemminger
@ 2019-06-24 16:47   ` Stephen Hemminger
  2019-06-24 16:47   ` [dpdk-dev] [PATCH v6 9/9] doc: update KNI documentation Stephen Hemminger
  2019-07-12 17:03   ` [dpdk-dev] [PATCH v6 0/9] kni: fixes and cleanups Ferruh Yigit
  9 siblings, 0 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-24 16:47 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

Some applications use ethtool so add the minimum ethtool ops.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 kernel/linux/kni/kni_dev.h  |  2 ++
 kernel/linux/kni/kni_misc.c |  1 +
 kernel/linux/kni/kni_net.c  | 14 ++++++++++++++
 3 files changed, 17 insertions(+)

diff --git a/kernel/linux/kni/kni_dev.h b/kernel/linux/kni/kni_dev.h
index ceba5f73c1d9..c1ca6789ce12 100644
--- a/kernel/linux/kni/kni_dev.h
+++ b/kernel/linux/kni/kni_dev.h
@@ -11,6 +11,8 @@
 #endif
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
+#define KNI_VERSION	"1.0"
+
 #include "compat.h"
 
 #include <linux/if.h>
diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c
index be45f823408f..2b75502a8b0e 100644
--- a/kernel/linux/kni/kni_misc.c
+++ b/kernel/linux/kni/kni_misc.c
@@ -21,6 +21,7 @@
 #include "compat.h"
 #include "kni_dev.h"
 
+MODULE_VERSION(KNI_VERSION);
 MODULE_LICENSE("Dual BSD/GPL");
 MODULE_AUTHOR("Intel Corporation");
 MODULE_DESCRIPTION("Kernel Module for managing kni devices");
diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c
index 320d51d7fc83..319ee2dcb19a 100644
--- a/kernel/linux/kni/kni_net.c
+++ b/kernel/linux/kni/kni_net.c
@@ -13,6 +13,7 @@
 #include <linux/version.h>
 #include <linux/netdevice.h>
 #include <linux/etherdevice.h> /* eth_type_trans */
+#include <linux/ethtool.h>
 #include <linux/skbuff.h>
 #include <linux/kthread.h>
 #include <linux/delay.h>
@@ -725,6 +726,18 @@ static const struct net_device_ops kni_net_netdev_ops = {
 #endif
 };
 
+static void kni_get_drvinfo(struct net_device *dev,
+			    struct ethtool_drvinfo *info)
+{
+	strlcpy(info->version, KNI_VERSION, sizeof(info->version));
+	strlcpy(info->driver, "kni", sizeof(info->driver));
+}
+
+static const struct ethtool_ops kni_net_ethtool_ops = {
+	.get_drvinfo	= kni_get_drvinfo,
+	.get_link	= ethtool_op_get_link,
+};
+
 void
 kni_net_init(struct net_device *dev)
 {
@@ -736,6 +749,7 @@ kni_net_init(struct net_device *dev)
 	ether_setup(dev); /* assign some of the fields */
 	dev->netdev_ops      = &kni_net_netdev_ops;
 	dev->header_ops      = &kni_net_header_ops;
+	dev->ethtool_ops     = &kni_net_ethtool_ops;
 	dev->watchdog_timeo = WD_TIMEOUT;
 }
 
-- 
2.20.1


^ permalink raw reply	[flat|nested] 63+ messages in thread

* [dpdk-dev] [PATCH v6 9/9] doc: update KNI documentation
  2019-06-24 16:47 ` [dpdk-dev] [PATCH v6 0/9] kni: fixes and cleanups Stephen Hemminger
                     ` (7 preceding siblings ...)
  2019-06-24 16:47   ` [dpdk-dev] [PATCH v6 8/9] kni: add minimal ethtool Stephen Hemminger
@ 2019-06-24 16:47   ` Stephen Hemminger
  2019-07-12 17:03   ` [dpdk-dev] [PATCH v6 0/9] kni: fixes and cleanups Ferruh Yigit
  9 siblings, 0 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-24 16:47 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

Update KNI documentation to reflect current ethtool support.

Replace references to out dated tools (ifconfig) with
modern iproute2.  Tshark is a better replacement for tcpdump.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 .../prog_guide/kernel_nic_interface.rst       |  9 +++----
 .../sample_app_ug/kernel_nic_interface.rst    | 24 ++++++++++++-------
 2 files changed, 20 insertions(+), 13 deletions(-)

diff --git a/doc/guides/prog_guide/kernel_nic_interface.rst b/doc/guides/prog_guide/kernel_nic_interface.rst
index daf87f4a8edf..5afd05c0a745 100644
--- a/doc/guides/prog_guide/kernel_nic_interface.rst
+++ b/doc/guides/prog_guide/kernel_nic_interface.rst
@@ -290,7 +290,8 @@ It then puts the mbuf back in the cache.
 Ethtool
 -------
 
-Ethtool is a Linux-specific tool with corresponding support in the kernel
-where each net device must register its own callbacks for the supported operations.
-The current implementation uses the igb/ixgbe modified Linux drivers for ethtool support.
-Ethtool is not supported in i40e and VMs (VF or EM devices).
+Ethtool is a Linux-specific tool with corresponding support in the kernel.
+The current version of kni provides minimal ethtool functionality
+including querying version and link state. It does not support link
+control, statistics, or dumping device registers.
+
diff --git a/doc/guides/sample_app_ug/kernel_nic_interface.rst b/doc/guides/sample_app_ug/kernel_nic_interface.rst
index a7e549d5213a..aac4ebd8d4ff 100644
--- a/doc/guides/sample_app_ug/kernel_nic_interface.rst
+++ b/doc/guides/sample_app_ug/kernel_nic_interface.rst
@@ -21,14 +21,14 @@ The FIFO queues contain pointers to data packets in the DPDK. This:
 
 *   Provides a faster mechanism to interface with the kernel net stack and eliminates system calls
 
-*   Facilitates the DPDK using standard Linux* userspace net tools (tcpdump, ftp, and so on)
+*   Facilitates the DPDK using standard Linux* userspace net tools (tshark, rsync, and so on)
 
 *   Eliminate the copy_to_user and copy_from_user operations on packets.
 
 The Kernel NIC Interface sample application is a simple example that demonstrates the use
 of the DPDK to create a path for packets to go through the Linux* kernel.
 This is done by creating one or more kernel net devices for each of the DPDK ports.
-The application allows the use of standard Linux tools (ethtool, ifconfig, tcpdump) with the DPDK ports and
+The application allows the use of standard Linux tools (ethtool, iproute, tshark) with the DPDK ports and
 also the exchange of packets between the DPDK application and the Linux* kernel.
 
 The Kernel NIC Interface sample application requires that the
@@ -220,13 +220,13 @@ Enable KNI interface and assign an IP address:
 
 .. code-block:: console
 
-    # ifconfig vEth0_0 192.168.0.1
+    # ip addr add dev vEth0_0 192.168.0.1
 
 Show KNI interface configuration and statistics:
 
 .. code-block:: console
 
-    # ifconfig vEth0_0
+    # ip -s -d addr show vEth0_0
 
 The user can also check and reset the packet statistics inside the ``kni``
 application by sending the app the USR1 and USR2 signals:
@@ -234,16 +234,16 @@ application by sending the app the USR1 and USR2 signals:
 .. code-block:: console
 
     # Print statistics
-    # kill -SIGUSR1 `pidof kni`
+    # pkill -USR1 kni
 
     # Zero statistics
-    # kill -SIGUSR2 `pidof kni`
+    # pkill -USR2 kni
 
 Dump network traffic:
 
 .. code-block:: console
 
-    # tcpdump -i vEth0_0
+    # tshark -n -i vEth0_0
 
 The normal Linux commands can also be used to change the MAC address and
 MTU size used by the physical NIC which corresponds to the KNI interface.
@@ -254,13 +254,19 @@ Change the MAC address:
 
 .. code-block:: console
 
-    # ifconfig vEth0_0 hw ether 0C:01:02:03:04:08
+    # ip link set dev vEth0_0 lladdr 0C:01:02:03:04:08
 
 Change the MTU size:
 
 .. code-block:: console
 
-    # ifconfig vEth0_0 mtu 1450
+    # ip link set dev vEth0_0 mtu 1450
+
+Limited ethtool support:
+
+.. code-block:: console
+
+    # ethtool -i vEth0_0
 
 When the ``kni`` application is closed, all the KNI interfaces are deleted
 from the Linux kernel.
-- 
2.20.1


^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [dpdk-dev] [PATCH v6 0/9] kni: fixes and cleanups
  2019-06-24 16:47 ` [dpdk-dev] [PATCH v6 0/9] kni: fixes and cleanups Stephen Hemminger
                     ` (8 preceding siblings ...)
  2019-06-24 16:47   ` [dpdk-dev] [PATCH v6 9/9] doc: update KNI documentation Stephen Hemminger
@ 2019-07-12 17:03   ` Ferruh Yigit
  2019-07-15 17:18     ` Thomas Monjalon
  9 siblings, 1 reply; 63+ messages in thread
From: Ferruh Yigit @ 2019-07-12 17:03 UTC (permalink / raw)
  To: Stephen Hemminger, Thomas Monjalon; +Cc: dev

On 6/24/2019 5:47 PM, Stephen Hemminger wrote:
> While testing KNI with netvsc, saw lots of places more code
> could be safely removed from KNI kernel driver.
> 
> v6 - more updates to documentation
> v5 - add minimal ethtool, fix checkpath author complaints
> v4 - add more style fixes
> v3 - rebase to current master, add style fix patch
> v2 - get rid of unnecessary padding, combine the unused field patches
> 
> Stephen Hemminger (9):
>   kni: don't need stubs for rx_mode or ioctl
>   kni: use netdev_alloc_skb
>   kni: don't keep stats in kni_net
>   kni: drop unused fields
>   kni: use proper type for kni fifo's
>   kni: return -EFAULT if copy_from_user fails
>   kni: fix style issues
>   kni: add minimal ethtool
>   doc: update KNI documentation

For series,
Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>

Thanks for the cleanup.

I am OK to get the set for rc2, but if we will do this please lets merge this
early so that we can still have time to test and fix any possible issues before rc2.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [dpdk-dev] [PATCH v6 0/9] kni: fixes and cleanups
  2019-07-12 17:03   ` [dpdk-dev] [PATCH v6 0/9] kni: fixes and cleanups Ferruh Yigit
@ 2019-07-15 17:18     ` Thomas Monjalon
  0 siblings, 0 replies; 63+ messages in thread
From: Thomas Monjalon @ 2019-07-15 17:18 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev, Ferruh Yigit

12/07/2019 19:03, Ferruh Yigit:
> On 6/24/2019 5:47 PM, Stephen Hemminger wrote:
> > While testing KNI with netvsc, saw lots of places more code
> > could be safely removed from KNI kernel driver.
> > 
> > v6 - more updates to documentation
> > v5 - add minimal ethtool, fix checkpath author complaints
> > v4 - add more style fixes
> > v3 - rebase to current master, add style fix patch
> > v2 - get rid of unnecessary padding, combine the unused field patches
> > 
> > Stephen Hemminger (9):
> >   kni: don't need stubs for rx_mode or ioctl
> >   kni: use netdev_alloc_skb
> >   kni: don't keep stats in kni_net
> >   kni: drop unused fields
> >   kni: use proper type for kni fifo's
> >   kni: return -EFAULT if copy_from_user fails
> >   kni: fix style issues
> >   kni: add minimal ethtool
> >   doc: update KNI documentation
> 
> For series,
> Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>
> 
> Thanks for the cleanup.
> 
> I am OK to get the set for rc2, but if we will do this please lets merge this
> early so that we can still have time to test and fix any possible issues before rc2.

Applied, thanks



^ permalink raw reply	[flat|nested] 63+ messages in thread

end of thread, other threads:[~2019-07-15 17:18 UTC | newest]

Thread overview: 63+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-10 17:51 [dpdk-dev] [PATCH v2 0/7] kni: cleanups and improvements Stephen Hemminger
2019-06-10 17:51 ` [dpdk-dev] [PATCH v2 1/7] kni: don't need stubs for rx_mode or ioctl Stephen Hemminger
2019-06-10 17:51 ` [dpdk-dev] [PATCH v2 2/7] kni: use netdev_alloc_skb Stephen Hemminger
2019-06-10 17:51 ` [dpdk-dev] [PATCH v2 3/7] kni: don't keep stats in kni_net Stephen Hemminger
2019-06-10 17:51 ` [dpdk-dev] [PATCH v2 4/7] kni: drop unused fields Stephen Hemminger
2019-06-10 17:51 ` [dpdk-dev] [PATCH v2 5/7] kni: use proper type for kni fifo's Stephen Hemminger
2019-06-10 17:51 ` [dpdk-dev] [PATCH v2 6/7] kni: return -EFAULT if copy_from_user fails Stephen Hemminger
2019-06-10 17:51 ` [dpdk-dev] [PATCH v2 7/7] doc: update KNI documentation Stephen Hemminger
2019-06-11 20:54 ` [dpdk-dev] [PATCH v2 0/7] kni: cleanups and improvements Stephen Hemminger
2019-06-11 21:18   ` Lance Richardson
2019-06-11 21:30     ` Stephen Hemminger
2019-06-11 21:49       ` Lance Richardson
2019-06-18 16:16 ` [dpdk-dev] [PATCH v3 0/8] kni: fixes and cleanups Stephen Hemminger
2019-06-18 16:16   ` [dpdk-dev] [PATCH v3 1/8] kni: don't need stubs for rx_mode or ioctl Stephen Hemminger
2019-06-18 16:16   ` [dpdk-dev] [PATCH v3 2/8] kni: use netdev_alloc_skb Stephen Hemminger
2019-06-18 16:16   ` [dpdk-dev] [PATCH v3 3/8] kni: don't keep stats in kni_net Stephen Hemminger
2019-06-18 16:16   ` [dpdk-dev] [PATCH v3 4/8] kni: drop unused fields Stephen Hemminger
2019-06-18 16:16   ` [dpdk-dev] [PATCH v3 5/8] kni: use proper type for kni fifo's Stephen Hemminger
2019-06-18 16:16   ` [dpdk-dev] [PATCH v3 6/8] kni: return -EFAULT if copy_from_user fails Stephen Hemminger
2019-06-18 16:16   ` [dpdk-dev] [PATCH v3 7/8] doc: update KNI documentation Stephen Hemminger
2019-06-18 16:16   ` [dpdk-dev] [PATCH v3 8/8] kni: fix style issues Stephen Hemminger
2019-06-19 18:57 ` [dpdk-dev] [PATCH v3 0/8] kni: cleanups and fixes Stephen Hemminger
2019-06-19 18:57   ` [dpdk-dev] [PATCH v3 1/8] kni: don't need stubs for rx_mode or ioctl Stephen Hemminger
2019-06-19 18:57   ` [dpdk-dev] [PATCH v3 2/8] kni: use netdev_alloc_skb Stephen Hemminger
2019-06-19 18:57   ` [dpdk-dev] [PATCH v3 3/8] kni: don't keep stats in kni_net Stephen Hemminger
2019-06-19 18:57   ` [dpdk-dev] [PATCH v3 4/8] kni: drop unused fields Stephen Hemminger
2019-06-19 18:57   ` [dpdk-dev] [PATCH v3 5/8] kni: use proper type for kni fifo's Stephen Hemminger
2019-06-19 18:57   ` [dpdk-dev] [PATCH v3 6/8] kni: return -EFAULT if copy_from_user fails Stephen Hemminger
2019-06-19 18:57   ` [dpdk-dev] [PATCH v3 7/8] doc: update KNI documentation Stephen Hemminger
2019-06-19 18:57   ` [dpdk-dev] [PATCH v3 8/8] kni: fix style issues Stephen Hemminger
2019-06-19 18:59 ` [dpdk-dev] [PATCH v4 0/8] kni: fixes and cleanups Stephen Hemminger
2019-06-19 18:59   ` [dpdk-dev] [PATCH v4 1/8] kni: don't need stubs for rx_mode or ioctl Stephen Hemminger
2019-06-19 18:59   ` [dpdk-dev] [PATCH v4 2/8] kni: use netdev_alloc_skb Stephen Hemminger
2019-06-19 18:59   ` [dpdk-dev] [PATCH v4 3/8] kni: don't keep stats in kni_net Stephen Hemminger
2019-06-19 18:59   ` [dpdk-dev] [PATCH v4 4/8] kni: drop unused fields Stephen Hemminger
2019-06-19 18:59   ` [dpdk-dev] [PATCH v4 5/8] kni: use proper type for kni fifo's Stephen Hemminger
2019-06-19 18:59   ` [dpdk-dev] [PATCH v4 6/8] kni: return -EFAULT if copy_from_user fails Stephen Hemminger
2019-06-19 18:59   ` [dpdk-dev] [PATCH v4 7/8] doc: update KNI documentation Stephen Hemminger
2019-06-19 18:59   ` [dpdk-dev] [PATCH v4 8/8] kni: fix style issues Stephen Hemminger
2019-06-20 19:20 ` [dpdk-dev] [PATCH v5 0/9] kni: fixes and cleanups Stephen Hemminger
2019-06-20 19:20   ` [dpdk-dev] [PATCH v5 1/9] kni: don't need stubs for rx_mode or ioctl Stephen Hemminger
2019-06-20 19:20   ` [dpdk-dev] [PATCH v5 2/9] kni: use netdev_alloc_skb Stephen Hemminger
2019-06-20 19:20   ` [dpdk-dev] [PATCH v5 3/9] kni: don't keep stats in kni_net Stephen Hemminger
2019-06-20 19:20   ` [dpdk-dev] [PATCH v5 4/9] kni: drop unused fields Stephen Hemminger
2019-06-20 19:20   ` [dpdk-dev] [PATCH v5 5/9] kni: use proper type for kni fifo's Stephen Hemminger
2019-06-20 19:20   ` [dpdk-dev] [PATCH v5 6/9] kni: return -EFAULT if copy_from_user fails Stephen Hemminger
2019-06-20 19:20   ` [dpdk-dev] [PATCH v5 7/9] doc: update KNI documentation Stephen Hemminger
2019-06-20 19:20   ` [dpdk-dev] [PATCH v5 8/9] kni: fix style issues Stephen Hemminger
2019-06-20 19:20   ` [dpdk-dev] [PATCH v5 9/9] kni: add minimal ethtool Stephen Hemminger
2019-06-21  8:26     ` Igor Ryzhov
2019-06-21 15:10       ` Stephen Hemminger
2019-06-24 16:47 ` [dpdk-dev] [PATCH v6 0/9] kni: fixes and cleanups Stephen Hemminger
2019-06-24 16:47   ` [dpdk-dev] [PATCH v6 1/9] kni: don't need stubs for rx_mode or ioctl Stephen Hemminger
2019-06-24 16:47   ` [dpdk-dev] [PATCH v6 2/9] kni: use netdev_alloc_skb Stephen Hemminger
2019-06-24 16:47   ` [dpdk-dev] [PATCH v6 3/9] kni: don't keep stats in kni_net Stephen Hemminger
2019-06-24 16:47   ` [dpdk-dev] [PATCH v6 4/9] kni: drop unused fields Stephen Hemminger
2019-06-24 16:47   ` [dpdk-dev] [PATCH v6 5/9] kni: use proper type for kni fifo's Stephen Hemminger
2019-06-24 16:47   ` [dpdk-dev] [PATCH v6 6/9] kni: return -EFAULT if copy_from_user fails Stephen Hemminger
2019-06-24 16:47   ` [dpdk-dev] [PATCH v6 7/9] kni: fix style issues Stephen Hemminger
2019-06-24 16:47   ` [dpdk-dev] [PATCH v6 8/9] kni: add minimal ethtool Stephen Hemminger
2019-06-24 16:47   ` [dpdk-dev] [PATCH v6 9/9] doc: update KNI documentation Stephen Hemminger
2019-07-12 17:03   ` [dpdk-dev] [PATCH v6 0/9] kni: fixes and cleanups Ferruh Yigit
2019-07-15 17:18     ` Thomas Monjalon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).