[dpdk-dev] [PATCH v2 0/7] kni: cleanups and improvements

[dpdk-dev] [PATCH v2 0/7] kni: cleanups and improvements

[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

[dpdk-dev] [PATCH v2 1/7] kni: don't need stubs for rx_mode or ioctl

[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

[dpdk-dev] [PATCH v2 2/7] kni: use netdev_alloc_skb

[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

[dpdk-dev] [PATCH v2 3/7] kni: don't keep stats in kni_net

[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

[dpdk-dev] [PATCH v2 4/7] kni: drop unused fields

[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

[dpdk-dev] [PATCH v2 5/7] kni: use proper type for kni fifo's

[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

[dpdk-dev] [PATCH v2 6/7] kni: return -EFAULT if copy_from_user fails

[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

[dpdk-dev] [PATCH v2 7/7] doc: update KNI documentation

[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

Re: [dpdk-dev] [PATCH v2 0/7] kni: cleanups and improvements

[Stephen Hemminger]

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.

Re: [dpdk-dev] [PATCH v2 0/7] kni: cleanups and improvements

[Lance Richardson]

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....

Re: [dpdk-dev] [PATCH v2 0/7] kni: cleanups and improvements

[Stephen Hemminger]

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

Re: [dpdk-dev] [PATCH v2 0/7] kni: cleanups and improvements

[Lance Richardson]

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

[dpdk-dev] [PATCH v3 0/8] kni: fixes and cleanups

[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

[dpdk-dev] [PATCH v3 1/8] kni: don't need stubs for rx_mode or ioctl

[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

[dpdk-dev] [PATCH v3 2/8] kni: use netdev_alloc_skb

[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

[dpdk-dev] [PATCH v3 3/8] kni: don't keep stats in kni_net

[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

[dpdk-dev] [PATCH v3 4/8] kni: drop unused fields

[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

[dpdk-dev] [PATCH v3 5/8] kni: use proper type for kni fifo's

[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

[dpdk-dev] [PATCH v3 6/8] kni: return -EFAULT if copy_from_user fails

[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

[dpdk-dev] [PATCH v3 7/8] doc: update KNI documentation

[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

[dpdk-dev] [PATCH v3 8/8] kni: fix style issues

[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

[dpdk-dev] [PATCH v3 0/8] kni: cleanups and fixes

[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

[dpdk-dev] [PATCH v3 1/8] kni: don't need stubs for rx_mode or ioctl

[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

[dpdk-dev] [PATCH v3 2/8] kni: use netdev_alloc_skb

[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

[dpdk-dev] [PATCH v3 3/8] kni: don't keep stats in kni_net

[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

[dpdk-dev] [PATCH v3 4/8] kni: drop unused fields

[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

[dpdk-dev] [PATCH v3 5/8] kni: use proper type for kni fifo's

[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

[dpdk-dev] [PATCH v3 6/8] kni: return -EFAULT if copy_from_user fails

[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

[dpdk-dev] [PATCH v3 7/8] doc: update KNI documentation

[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

[dpdk-dev] [PATCH v3 8/8] kni: fix style issues

[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

[dpdk-dev] [PATCH v4 0/8] kni: fixes and cleanups

[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

[dpdk-dev] [PATCH v4 1/8] kni: don't need stubs for rx_mode or ioctl

[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

[dpdk-dev] [PATCH v4 2/8] kni: use netdev_alloc_skb

[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

[dpdk-dev] [PATCH v4 3/8] kni: don't keep stats in kni_net

[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

[dpdk-dev] [PATCH v4 4/8] kni: drop unused fields

[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

[dpdk-dev] [PATCH v4 5/8] kni: use proper type for kni fifo's

[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

[dpdk-dev] [PATCH v4 6/8] kni: return -EFAULT if copy_from_user fails

[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

[dpdk-dev] [PATCH v4 7/8] doc: update KNI documentation

[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

[dpdk-dev] [PATCH v4 8/8] kni: fix style issues

[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

[dpdk-dev] [PATCH v5 0/9] kni: fixes and cleanups

[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

[dpdk-dev] [PATCH v5 1/9] kni: don't need stubs for rx_mode or ioctl

[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

[dpdk-dev] [PATCH v5 2/9] kni: use netdev_alloc_skb

[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

[dpdk-dev] [PATCH v5 3/9] kni: don't keep stats in kni_net

[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

[dpdk-dev] [PATCH v5 4/9] kni: drop unused fields

[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

[dpdk-dev] [PATCH v5 5/9] kni: use proper type for kni fifo's

[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

[dpdk-dev] [PATCH v5 6/9] kni: return -EFAULT if copy_from_user fails

[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

[dpdk-dev] [PATCH v5 7/9] doc: update KNI documentation

[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

[dpdk-dev] [PATCH v5 8/9] kni: fix style issues

[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

[dpdk-dev] [PATCH v5 9/9] kni: add minimal ethtool

[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

Re: [dpdk-dev] [PATCH v5 9/9] kni: add minimal ethtool

[Igor Ryzhov]

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
>

Re: [dpdk-dev] [PATCH v5 9/9] kni: add minimal ethtool

[Stephen Hemminger]

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).

[dpdk-dev] [PATCH v6 0/9] kni: fixes and cleanups

[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

[dpdk-dev] [PATCH v6 1/9] kni: don't need stubs for rx_mode or ioctl

[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

[dpdk-dev] [PATCH v6 2/9] kni: use netdev_alloc_skb

[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

[dpdk-dev] [PATCH v6 3/9] kni: don't keep stats in kni_net

[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

[dpdk-dev] [PATCH v6 4/9] kni: drop unused fields

[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

[dpdk-dev] [PATCH v6 5/9] kni: use proper type for kni fifo's

[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

[dpdk-dev] [PATCH v6 6/9] kni: return -EFAULT if copy_from_user fails

[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

[dpdk-dev] [PATCH v6 7/9] kni: fix style issues

[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

[dpdk-dev] [PATCH v6 8/9] kni: add minimal ethtool

[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

[dpdk-dev] [PATCH v6 9/9] doc: update KNI documentation

[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

Re: [dpdk-dev] [PATCH v6 0/9] kni: fixes and cleanups

[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.

Re: [dpdk-dev] [PATCH v6 0/9] kni: fixes and cleanups

[Thomas Monjalon]

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