* [dpdk-dev] [PATCH v2 0/7] kni: cleanups and improvements
@ 2019-06-10 17:51 Stephen Hemminger
2019-06-10 17:51 ` [dpdk-dev] [PATCH v2 1/7] kni: don't need stubs for rx_mode or ioctl Stephen Hemminger
` (12 more replies)
0 siblings, 13 replies; 63+ messages in thread
From: Stephen Hemminger @ 2019-06-10 17:51 UTC (permalink / raw)
To: ferruh.yigit; +Cc: dev, Stephen Hemminger
While testing KNI with netvsc, saw lots of places more code
could be safely removed from KNI kernel driver.
This is still mostly "putting lipstick on a pig" all users
would be better off using virtio_user rather than KNI.
v2 - get rid of unnecessary padding, combine the unused field patches
Stephen Hemminger (7):
kni: don't need stubs for rx_mode or ioctl
kni: use netdev_alloc_skb
kni: don't keep stats in kni_net
kni: drop unused fields
kni: use proper type for kni fifo's
kni: return -EFAULT if copy_from_user fails
doc: update KNI documentation
.../sample_app_ug/kernel_nic_interface.rst | 18 ++---
kernel/linux/kni/kni_dev.h | 21 ++---
kernel/linux/kni/kni_misc.c | 17 ++--
kernel/linux/kni/kni_net.c | 79 +++++--------------
4 files changed, 38 insertions(+), 97 deletions(-)
--
2.20.1
^ permalink raw reply [flat|nested] 63+ messages in thread* [dpdk-dev] [PATCH v2 1/7] kni: don't need stubs for rx_mode or ioctl 2019-06-10 17:51 [dpdk-dev] [PATCH v2 0/7] kni: cleanups and improvements Stephen Hemminger @ 2019-06-10 17:51 ` Stephen Hemminger 2019-06-10 17:51 ` [dpdk-dev] [PATCH v2 2/7] kni: use netdev_alloc_skb Stephen Hemminger ` (11 subsequent siblings) 12 siblings, 0 replies; 63+ messages in thread From: Stephen Hemminger @ 2019-06-10 17:51 UTC (permalink / raw) To: ferruh.yigit; +Cc: dev, Stephen Hemminger The netdev subsystem already handles case where network sevice does not support ioctl. If device has no rx_mode hook it is not called. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- kernel/linux/kni/kni_net.c | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c index ad8365877cda..c86337d099ab 100644 --- a/kernel/linux/kni/kni_net.c +++ b/kernel/linux/kni/kni_net.c @@ -593,23 +593,6 @@ kni_net_tx_timeout(struct net_device *dev) netif_wake_queue(dev); } -/* - * Ioctl commands - */ -static int -kni_net_ioctl(struct net_device *dev, struct ifreq *rq, int cmd) -{ - pr_debug("kni_net_ioctl group:%d cmd:%d\n", - ((struct kni_dev *)netdev_priv(dev))->group_id, cmd); - - return -EOPNOTSUPP; -} - -static void -kni_net_set_rx_mode(struct net_device *dev) -{ -} - static int kni_net_change_mtu(struct net_device *dev, int new_mtu) { @@ -758,8 +741,6 @@ static const struct net_device_ops kni_net_netdev_ops = { .ndo_change_rx_flags = kni_net_set_promiscusity, .ndo_start_xmit = kni_net_tx, .ndo_change_mtu = kni_net_change_mtu, - .ndo_do_ioctl = kni_net_ioctl, - .ndo_set_rx_mode = kni_net_set_rx_mode, .ndo_get_stats = kni_net_stats, .ndo_tx_timeout = kni_net_tx_timeout, .ndo_set_mac_address = kni_net_set_mac, -- 2.20.1 ^ permalink raw reply [flat|nested] 63+ messages in thread
* [dpdk-dev] [PATCH v2 2/7] kni: use netdev_alloc_skb 2019-06-10 17:51 [dpdk-dev] [PATCH v2 0/7] kni: cleanups and improvements Stephen Hemminger 2019-06-10 17:51 ` [dpdk-dev] [PATCH v2 1/7] kni: don't need stubs for rx_mode or ioctl Stephen Hemminger @ 2019-06-10 17:51 ` Stephen Hemminger 2019-06-10 17:51 ` [dpdk-dev] [PATCH v2 3/7] kni: don't keep stats in kni_net Stephen Hemminger ` (10 subsequent siblings) 12 siblings, 0 replies; 63+ messages in thread From: Stephen Hemminger @ 2019-06-10 17:51 UTC (permalink / raw) To: ferruh.yigit; +Cc: dev, Stephen Hemminger netdev_alloc_skb is optimized to any alignment or setup of skb->dev that is required. The kernel intentionall does not pad packets on x86, because it is faster. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- kernel/linux/kni/kni_net.c | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c index c86337d099ab..cce5e7eb991f 100644 --- a/kernel/linux/kni/kni_net.c +++ b/kernel/linux/kni/kni_net.c @@ -340,16 +340,13 @@ kni_net_rx_normal(struct kni_dev *kni) data_kva = kva2data_kva(kva); kni->va[i] = pa2va(kni->pa[i], kva); - skb = dev_alloc_skb(len + 2); + skb = netdev_alloc_skb(dev, len); if (!skb) { /* Update statistics */ kni->stats.rx_dropped++; continue; } - /* Align IP on 16B boundary */ - skb_reserve(skb, 2); - if (kva->nb_segs == 1) { memcpy(skb_put(skb, len), data_kva, len); } else { @@ -368,7 +365,6 @@ kni_net_rx_normal(struct kni_dev *kni) } } - skb->dev = dev; skb->protocol = eth_type_trans(skb, dev); skb->ip_summed = CHECKSUM_UNNECESSARY; @@ -512,26 +508,20 @@ kni_net_rx_lo_fifo_skb(struct kni_dev *kni) data_kva = kva2data_kva(kva); kni->va[i] = pa2va(kni->pa[i], kva); - skb = dev_alloc_skb(len + 2); + skb = netdev_alloc_skb(dev, len); if (skb) { - /* Align IP on 16B boundary */ - skb_reserve(skb, 2); memcpy(skb_put(skb, len), data_kva, len); - skb->dev = dev; skb->ip_summed = CHECKSUM_UNNECESSARY; dev_kfree_skb(skb); } /* Simulate real usage, allocate/copy skb twice */ - skb = dev_alloc_skb(len + 2); + skb = netdev_alloc_skb(dev, len); if (skb == NULL) { kni->stats.rx_dropped++; continue; } - /* Align IP on 16B boundary */ - skb_reserve(skb, 2); - if (kva->nb_segs == 1) { memcpy(skb_put(skb, len), data_kva, len); } else { @@ -550,7 +540,6 @@ kni_net_rx_lo_fifo_skb(struct kni_dev *kni) } } - skb->dev = dev; skb->ip_summed = CHECKSUM_UNNECESSARY; kni->stats.rx_bytes += len; -- 2.20.1 ^ permalink raw reply [flat|nested] 63+ messages in thread
* [dpdk-dev] [PATCH v2 3/7] kni: don't keep stats in kni_net 2019-06-10 17:51 [dpdk-dev] [PATCH v2 0/7] kni: cleanups and improvements Stephen Hemminger 2019-06-10 17:51 ` [dpdk-dev] [PATCH v2 1/7] kni: don't need stubs for rx_mode or ioctl Stephen Hemminger 2019-06-10 17:51 ` [dpdk-dev] [PATCH v2 2/7] kni: use netdev_alloc_skb Stephen Hemminger @ 2019-06-10 17:51 ` Stephen Hemminger 2019-06-10 17:51 ` [dpdk-dev] [PATCH v2 4/7] kni: drop unused fields Stephen Hemminger ` (9 subsequent siblings) 12 siblings, 0 replies; 63+ messages in thread From: Stephen Hemminger @ 2019-06-10 17:51 UTC (permalink / raw) To: ferruh.yigit; +Cc: dev, Stephen Hemminger Since kernel 2.6.28 the network subsystem has provided dev->stats for devices to use statistics handling and is the default if no ndo_get_stats is provided. This allow allows for 64 bit (rather than just 32 bit) statistics with KNI. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- kernel/linux/kni/kni_dev.h | 1 - kernel/linux/kni/kni_net.c | 43 +++++++++++++------------------------- 2 files changed, 15 insertions(+), 29 deletions(-) diff --git a/kernel/linux/kni/kni_dev.h b/kernel/linux/kni/kni_dev.h index 44a5a61f7279..f3a1f60d4bdc 100644 --- a/kernel/linux/kni/kni_dev.h +++ b/kernel/linux/kni/kni_dev.h @@ -39,7 +39,6 @@ struct kni_dev { /* kni list */ struct list_head list; - struct net_device_stats stats; int status; uint16_t group_id; /* Group ID of a group of KNI devices */ uint32_t core_id; /* Core ID to bind */ diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c index cce5e7eb991f..06310fec57bb 100644 --- a/kernel/linux/kni/kni_net.c +++ b/kernel/linux/kni/kni_net.c @@ -291,15 +291,15 @@ kni_net_tx(struct sk_buff *skb, struct net_device *dev) /* Free skb and update statistics */ dev_kfree_skb(skb); - kni->stats.tx_bytes += len; - kni->stats.tx_packets++; + dev->stats.tx_bytes += len; + dev->stats.tx_packets++; return NETDEV_TX_OK; drop: /* Free skb and update statistics */ dev_kfree_skb(skb); - kni->stats.tx_dropped++; + dev->stats.tx_dropped++; return NETDEV_TX_OK; } @@ -343,7 +343,7 @@ kni_net_rx_normal(struct kni_dev *kni) skb = netdev_alloc_skb(dev, len); if (!skb) { /* Update statistics */ - kni->stats.rx_dropped++; + dev->stats.rx_dropped++; continue; } @@ -372,8 +372,8 @@ kni_net_rx_normal(struct kni_dev *kni) netif_rx_ni(skb); /* Update statistics */ - kni->stats.rx_bytes += len; - kni->stats.rx_packets++; + dev->stats.rx_bytes += len; + dev->stats.rx_packets++; } /* Burst enqueue mbufs into free_q */ @@ -396,6 +396,7 @@ kni_net_rx_lo_fifo(struct kni_dev *kni) void *data_kva; struct rte_kni_mbuf *alloc_kva; void *alloc_data_kva; + struct net_device *dev = kni->net_dev; /* Get the number of entries in rx_q */ num_rq = kni_fifo_count(kni->rx_q); @@ -443,8 +444,8 @@ kni_net_rx_lo_fifo(struct kni_dev *kni) alloc_kva->pkt_len = len; alloc_kva->data_len = len; - kni->stats.tx_bytes += len; - kni->stats.rx_bytes += len; + dev->stats.tx_bytes += len; + dev->stats.rx_bytes += len; } /* Burst enqueue mbufs into tx_q */ @@ -464,8 +465,8 @@ kni_net_rx_lo_fifo(struct kni_dev *kni) * Update statistic, and enqueue/dequeue failure is impossible, * as all queues are checked at first. */ - kni->stats.tx_packets += num; - kni->stats.rx_packets += num; + dev->stats.tx_packets += num; + dev->stats.rx_packets += num; } /* @@ -518,7 +519,7 @@ kni_net_rx_lo_fifo_skb(struct kni_dev *kni) /* Simulate real usage, allocate/copy skb twice */ skb = netdev_alloc_skb(dev, len); if (skb == NULL) { - kni->stats.rx_dropped++; + dev->stats.rx_dropped++; continue; } @@ -542,8 +543,8 @@ kni_net_rx_lo_fifo_skb(struct kni_dev *kni) skb->ip_summed = CHECKSUM_UNNECESSARY; - kni->stats.rx_bytes += len; - kni->stats.rx_packets++; + dev->stats.rx_bytes += len; + dev->stats.rx_packets++; /* call tx interface */ kni_net_tx(skb, dev); @@ -573,12 +574,10 @@ kni_net_rx(struct kni_dev *kni) static void kni_net_tx_timeout(struct net_device *dev) { - struct kni_dev *kni = netdev_priv(dev); - pr_debug("Transmit timeout at %ld, latency %ld\n", jiffies, jiffies - dev_trans_start(dev)); - kni->stats.tx_errors++; + dev->stats.tx_errors++; netif_wake_queue(dev); } @@ -627,17 +626,6 @@ kni_net_poll_resp(struct kni_dev *kni) wake_up_interruptible(&kni->wq); } -/* - * Return statistics to the caller - */ -static struct net_device_stats * -kni_net_stats(struct net_device *dev) -{ - struct kni_dev *kni = netdev_priv(dev); - - return &kni->stats; -} - /* * Fill the eth header */ @@ -730,7 +718,6 @@ static const struct net_device_ops kni_net_netdev_ops = { .ndo_change_rx_flags = kni_net_set_promiscusity, .ndo_start_xmit = kni_net_tx, .ndo_change_mtu = kni_net_change_mtu, - .ndo_get_stats = kni_net_stats, .ndo_tx_timeout = kni_net_tx_timeout, .ndo_set_mac_address = kni_net_set_mac, #ifdef HAVE_CHANGE_CARRIER_CB -- 2.20.1 ^ permalink raw reply [flat|nested] 63+ messages in thread
* [dpdk-dev] [PATCH v2 4/7] kni: drop unused fields 2019-06-10 17:51 [dpdk-dev] [PATCH v2 0/7] kni: cleanups and improvements Stephen Hemminger ` (2 preceding siblings ...) 2019-06-10 17:51 ` [dpdk-dev] [PATCH v2 3/7] kni: don't keep stats in kni_net Stephen Hemminger @ 2019-06-10 17:51 ` Stephen Hemminger 2019-06-10 17:51 ` [dpdk-dev] [PATCH v2 5/7] kni: use proper type for kni fifo's Stephen Hemminger ` (8 subsequent siblings) 12 siblings, 0 replies; 63+ messages in thread From: Stephen Hemminger @ 2019-06-10 17:51 UTC (permalink / raw) To: ferruh.yigit; +Cc: dev, Stephen Hemminger The kni net structure only exists in driver no API/ABI. Several fields were either totally unused or set and never used. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- kernel/linux/kni/kni_dev.h | 8 -------- kernel/linux/kni/kni_misc.c | 1 - 2 files changed, 9 deletions(-) diff --git a/kernel/linux/kni/kni_dev.h b/kernel/linux/kni/kni_dev.h index f3a1f60d4bdc..f3e6c3ca4efa 100644 --- a/kernel/linux/kni/kni_dev.h +++ b/kernel/linux/kni/kni_dev.h @@ -39,8 +39,6 @@ struct kni_dev { /* kni list */ struct list_head list; - int status; - uint16_t group_id; /* Group ID of a group of KNI devices */ uint32_t core_id; /* Core ID to bind */ char name[RTE_KNI_NAMESIZE]; /* Network device name */ struct task_struct *pthread; @@ -49,9 +47,6 @@ struct kni_dev { wait_queue_head_t wq; struct mutex sync_lock; - /* PCI device id */ - uint16_t device_id; - /* kni device */ struct net_device *net_dev; @@ -82,9 +77,6 @@ struct kni_dev { /* mbuf size */ uint32_t mbuf_size; - /* synchro for request processing */ - unsigned long synchro; - /* buffers */ void *pa[MBUF_BURST_SZ]; void *va[MBUF_BURST_SZ]; diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c index af18c67c422f..6a206d883c0d 100644 --- a/kernel/linux/kni/kni_misc.c +++ b/kernel/linux/kni/kni_misc.c @@ -346,7 +346,6 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num, kni = netdev_priv(net_dev); kni->net_dev = net_dev; - kni->group_id = dev_info.group_id; kni->core_id = dev_info.core_id; strncpy(kni->name, dev_info.name, RTE_KNI_NAMESIZE); -- 2.20.1 ^ permalink raw reply [flat|nested] 63+ messages in thread
* [dpdk-dev] [PATCH v2 5/7] kni: use proper type for kni fifo's 2019-06-10 17:51 [dpdk-dev] [PATCH v2 0/7] kni: cleanups and improvements Stephen Hemminger ` (3 preceding siblings ...) 2019-06-10 17:51 ` [dpdk-dev] [PATCH v2 4/7] kni: drop unused fields Stephen Hemminger @ 2019-06-10 17:51 ` Stephen Hemminger 2019-06-10 17:51 ` [dpdk-dev] [PATCH v2 6/7] kni: return -EFAULT if copy_from_user fails Stephen Hemminger ` (7 subsequent siblings) 12 siblings, 0 replies; 63+ messages in thread From: Stephen Hemminger @ 2019-06-10 17:51 UTC (permalink / raw) To: ferruh.yigit; +Cc: dev, Stephen Hemminger Using void * instead of proper type is unsafe practice. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- kernel/linux/kni/kni_dev.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/kernel/linux/kni/kni_dev.h b/kernel/linux/kni/kni_dev.h index f3e6c3ca4efa..ceba5f73c1d9 100644 --- a/kernel/linux/kni/kni_dev.h +++ b/kernel/linux/kni/kni_dev.h @@ -51,22 +51,22 @@ struct kni_dev { struct net_device *net_dev; /* queue for packets to be sent out */ - void *tx_q; + struct rte_kni_fifo *tx_q; /* queue for the packets received */ - void *rx_q; + struct rte_kni_fifo *rx_q; /* queue for the allocated mbufs those can be used to save sk buffs */ - void *alloc_q; + struct rte_kni_fifo *alloc_q; /* free queue for the mbufs to be freed */ - void *free_q; + struct rte_kni_fifo *free_q; /* request queue */ - void *req_q; + struct rte_kni_fifo *req_q; /* response queue */ - void *resp_q; + struct rte_kni_fifo *resp_q; void *sync_kva; void *sync_va; -- 2.20.1 ^ permalink raw reply [flat|nested] 63+ messages in thread
* [dpdk-dev] [PATCH v2 6/7] kni: return -EFAULT if copy_from_user fails 2019-06-10 17:51 [dpdk-dev] [PATCH v2 0/7] kni: cleanups and improvements Stephen Hemminger ` (4 preceding siblings ...) 2019-06-10 17:51 ` [dpdk-dev] [PATCH v2 5/7] kni: use proper type for kni fifo's Stephen Hemminger @ 2019-06-10 17:51 ` Stephen Hemminger 2019-06-10 17:51 ` [dpdk-dev] [PATCH v2 7/7] doc: update KNI documentation Stephen Hemminger ` (6 subsequent siblings) 12 siblings, 0 replies; 63+ messages in thread From: Stephen Hemminger @ 2019-06-10 17:51 UTC (permalink / raw) To: ferruh.yigit; +Cc: dev, Stephen Hemminger The correct thing to return if user gives a bad data is to return -EFAULT. Logging is also discouraged because it could be used as a DoS attack. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- kernel/linux/kni/kni_misc.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c index 6a206d883c0d..a67991066cd9 100644 --- a/kernel/linux/kni/kni_misc.c +++ b/kernel/linux/kni/kni_misc.c @@ -301,11 +301,8 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num, return -EINVAL; /* Copy kni info from user space */ - ret = copy_from_user(&dev_info, (void *)ioctl_param, sizeof(dev_info)); - if (ret) { - pr_err("copy_from_user in kni_ioctl_create"); - return -EIO; - } + if (copy_from_user(&dev_info, (void *)ioctl_param, sizeof(dev_info))) + return -EFAULT; /* Check if name is zero-ended */ if (strnlen(dev_info.name, sizeof(dev_info.name)) == sizeof(dev_info.name)) { @@ -433,15 +430,12 @@ kni_ioctl_release(struct net *net, uint32_t ioctl_num, if (_IOC_SIZE(ioctl_num) > sizeof(dev_info)) return -EINVAL; - ret = copy_from_user(&dev_info, (void *)ioctl_param, sizeof(dev_info)); - if (ret) { - pr_err("copy_from_user in kni_ioctl_release"); - return -EIO; - } + if (copy_from_user(&dev_info, (void *)ioctl_param, sizeof(dev_info))) + return -EFAULT; /* Release the network device according to its name */ if (strlen(dev_info.name) == 0) - return ret; + return -EINVAL; down_write(&knet->kni_list_lock); list_for_each_entry_safe(dev, n, &knet->kni_list_head, list) { -- 2.20.1 ^ permalink raw reply [flat|nested] 63+ messages in thread
* [dpdk-dev] [PATCH v2 7/7] doc: update KNI documentation 2019-06-10 17:51 [dpdk-dev] [PATCH v2 0/7] kni: cleanups and improvements Stephen Hemminger ` (5 preceding siblings ...) 2019-06-10 17:51 ` [dpdk-dev] [PATCH v2 6/7] kni: return -EFAULT if copy_from_user fails Stephen Hemminger @ 2019-06-10 17:51 ` Stephen Hemminger 2019-06-11 20:54 ` [dpdk-dev] [PATCH v2 0/7] kni: cleanups and improvements Stephen Hemminger ` (5 subsequent siblings) 12 siblings, 0 replies; 63+ messages in thread From: Stephen Hemminger @ 2019-06-10 17:51 UTC (permalink / raw) To: ferruh.yigit; +Cc: dev, Stephen Hemminger Make the KNI documentation reflect modern kernel networking. Ifconfig has been superseded by iproute2 for 15 years. Iproute2 is well maintained, supports current feature set. Ethtool is no longer supported by KNI. Tshark is a better replacement for tcpdump. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- .../sample_app_ug/kernel_nic_interface.rst | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/doc/guides/sample_app_ug/kernel_nic_interface.rst b/doc/guides/sample_app_ug/kernel_nic_interface.rst index a7e549d5213a..422bd8c98465 100644 --- a/doc/guides/sample_app_ug/kernel_nic_interface.rst +++ b/doc/guides/sample_app_ug/kernel_nic_interface.rst @@ -21,14 +21,14 @@ The FIFO queues contain pointers to data packets in the DPDK. This: * Provides a faster mechanism to interface with the kernel net stack and eliminates system calls -* Facilitates the DPDK using standard Linux* userspace net tools (tcpdump, ftp, and so on) +* Facilitates the DPDK using standard Linux* userspace net tools (tshark, rsync, and so on) * Eliminate the copy_to_user and copy_from_user operations on packets. The Kernel NIC Interface sample application is a simple example that demonstrates the use of the DPDK to create a path for packets to go through the Linux* kernel. This is done by creating one or more kernel net devices for each of the DPDK ports. -The application allows the use of standard Linux tools (ethtool, ifconfig, tcpdump) with the DPDK ports and +The application allows the use of standard Linux tools (iproute, tshark) with the DPDK ports and also the exchange of packets between the DPDK application and the Linux* kernel. The Kernel NIC Interface sample application requires that the @@ -220,13 +220,13 @@ Enable KNI interface and assign an IP address: .. code-block:: console - # ifconfig vEth0_0 192.168.0.1 + # ip addr add dev vEth0_0 192.168.0.1 Show KNI interface configuration and statistics: .. code-block:: console - # ifconfig vEth0_0 + # ip -s -d addr show vEth0_0 The user can also check and reset the packet statistics inside the ``kni`` application by sending the app the USR1 and USR2 signals: @@ -234,16 +234,16 @@ application by sending the app the USR1 and USR2 signals: .. code-block:: console # Print statistics - # kill -SIGUSR1 `pidof kni` + # pkill -USR1 kni # Zero statistics - # kill -SIGUSR2 `pidof kni` + # pkill -USR2 kni Dump network traffic: .. code-block:: console - # tcpdump -i vEth0_0 + # tshark -n -i vEth0_0 The normal Linux commands can also be used to change the MAC address and MTU size used by the physical NIC which corresponds to the KNI interface. @@ -254,13 +254,13 @@ Change the MAC address: .. code-block:: console - # ifconfig vEth0_0 hw ether 0C:01:02:03:04:08 + # ip link set dev vEth0_0 lladdr 0C:01:02:03:04:08 Change the MTU size: .. code-block:: console - # ifconfig vEth0_0 mtu 1450 + # ip link set dev vEth0_0 mtu 1450 When the ``kni`` application is closed, all the KNI interfaces are deleted from the Linux kernel. -- 2.20.1 ^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [dpdk-dev] [PATCH v2 0/7] kni: cleanups and improvements 2019-06-10 17:51 [dpdk-dev] [PATCH v2 0/7] kni: cleanups and improvements Stephen Hemminger ` (6 preceding siblings ...) 2019-06-10 17:51 ` [dpdk-dev] [PATCH v2 7/7] doc: update KNI documentation Stephen Hemminger @ 2019-06-11 20:54 ` Stephen Hemminger 2019-06-11 21:18 ` Lance Richardson 2019-06-18 16:16 ` [dpdk-dev] [PATCH v3 0/8] kni: fixes and cleanups Stephen Hemminger ` (4 subsequent siblings) 12 siblings, 1 reply; 63+ messages in thread From: Stephen Hemminger @ 2019-06-11 20:54 UTC (permalink / raw) To: ferruh.yigit; +Cc: dev On Mon, 10 Jun 2019 10:51:48 -0700 Stephen Hemminger <stephen@networkplumber.org> wrote: > While testing KNI with netvsc, saw lots of places more code > could be safely removed from KNI kernel driver. > > This is still mostly "putting lipstick on a pig" all users > would be better off using virtio_user rather than KNI. > > v2 - get rid of unnecessary padding, combine the unused field patches > > Stephen Hemminger (7): > kni: don't need stubs for rx_mode or ioctl > kni: use netdev_alloc_skb > kni: don't keep stats in kni_net > kni: drop unused fields > kni: use proper type for kni fifo's > kni: return -EFAULT if copy_from_user fails > doc: update KNI documentation > > .../sample_app_ug/kernel_nic_interface.rst | 18 ++--- > kernel/linux/kni/kni_dev.h | 21 ++--- > kernel/linux/kni/kni_misc.c | 17 ++-- > kernel/linux/kni/kni_net.c | 79 +++++-------------- > 4 files changed, 38 insertions(+), 97 deletions(-) > Don't believe patchwork the patch is fine, it is getting falsely blamed for failures caused by other changes in ice, bnxt which fail FreeBSD build. ^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [dpdk-dev] [PATCH v2 0/7] kni: cleanups and improvements 2019-06-11 20:54 ` [dpdk-dev] [PATCH v2 0/7] kni: cleanups and improvements Stephen Hemminger @ 2019-06-11 21:18 ` Lance Richardson 2019-06-11 21:30 ` Stephen Hemminger 0 siblings, 1 reply; 63+ messages in thread From: Lance Richardson @ 2019-06-11 21:18 UTC (permalink / raw) To: Stephen Hemminger; +Cc: Ferruh Yigit, dev On Tue, Jun 11, 2019 at 4:54 PM Stephen Hemminger <stephen@networkplumber.org> wrote: > > On Mon, 10 Jun 2019 10:51:48 -0700 > Stephen Hemminger <stephen@networkplumber.org> wrote: > > > While testing KNI with netvsc, saw lots of places more code > > could be safely removed from KNI kernel driver. > > > > This is still mostly "putting lipstick on a pig" all users > > would be better off using virtio_user rather than KNI. > > > > v2 - get rid of unnecessary padding, combine the unused field patches > > > > Stephen Hemminger (7): > > kni: don't need stubs for rx_mode or ioctl > > kni: use netdev_alloc_skb > > kni: don't keep stats in kni_net > > kni: drop unused fields > > kni: use proper type for kni fifo's > > kni: return -EFAULT if copy_from_user fails > > doc: update KNI documentation > > > > .../sample_app_ug/kernel_nic_interface.rst | 18 ++--- > > kernel/linux/kni/kni_dev.h | 21 ++--- > > kernel/linux/kni/kni_misc.c | 17 ++-- > > kernel/linux/kni/kni_net.c | 79 +++++-------------- > > 4 files changed, 38 insertions(+), 97 deletions(-) > > > > Don't believe patchwork the patch is fine, it is getting falsely blamed > for failures caused by other changes in ice, bnxt which fail > FreeBSD build. Do you mean failures like the ones below? If so, I think think they might be an unintended consequence of commit a385972c3 "mk: disable warning for packed member pointer". Lance OS: FreeBSD12-64 Target: x86_64-native-bsdapp-gcc CC ice_rxtx.o CC ice_ethdev.o cc1: error: unrecognized command line option '-Wno-address-of-packed-member' [-Werror] cc1: all warnings being treated as errors gmake[6]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc/e6c3bc062b1045128114bdf0cfdd236b/dpdk/mk/internal/rte.compile-pre.mk:114: bnxt_ethdev.o] Error 1 gmake[5]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc/e6c3bc062b1045128114bdf0cfdd236b/dpdk/mk/rte.subdir.mk:37: bnxt] Error 2 gmake[5]: *** Waiting for unfinished jobs.... ^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [dpdk-dev] [PATCH v2 0/7] kni: cleanups and improvements 2019-06-11 21:18 ` Lance Richardson @ 2019-06-11 21:30 ` Stephen Hemminger 2019-06-11 21:49 ` Lance Richardson 0 siblings, 1 reply; 63+ messages in thread From: Stephen Hemminger @ 2019-06-11 21:30 UTC (permalink / raw) To: Lance Richardson; +Cc: Ferruh Yigit, dev On Tue, 11 Jun 2019 17:18:42 -0400 Lance Richardson <lance.richardson@broadcom.com> wrote: > On Tue, Jun 11, 2019 at 4:54 PM Stephen Hemminger > <stephen@networkplumber.org> wrote: > > > > On Mon, 10 Jun 2019 10:51:48 -0700 > > Stephen Hemminger <stephen@networkplumber.org> wrote: > > > > > While testing KNI with netvsc, saw lots of places more code > > > could be safely removed from KNI kernel driver. > > > > > > This is still mostly "putting lipstick on a pig" all users > > > would be better off using virtio_user rather than KNI. > > > > > > v2 - get rid of unnecessary padding, combine the unused field patches > > > > > > Stephen Hemminger (7): > > > kni: don't need stubs for rx_mode or ioctl > > > kni: use netdev_alloc_skb > > > kni: don't keep stats in kni_net > > > kni: drop unused fields > > > kni: use proper type for kni fifo's > > > kni: return -EFAULT if copy_from_user fails > > > doc: update KNI documentation > > > > > > .../sample_app_ug/kernel_nic_interface.rst | 18 ++--- > > > kernel/linux/kni/kni_dev.h | 21 ++--- > > > kernel/linux/kni/kni_misc.c | 17 ++-- > > > kernel/linux/kni/kni_net.c | 79 +++++-------------- > > > 4 files changed, 38 insertions(+), 97 deletions(-) > > > > > > > Don't believe patchwork the patch is fine, it is getting falsely blamed > > for failures caused by other changes in ice, bnxt which fail > > FreeBSD build. > > Do you mean failures like the ones below? If so, I think think they > might be an unintended > consequence of commit a385972c3 "mk: disable warning for packed member pointer". > > Lance > > OS: FreeBSD12-64 > Target: x86_64-native-bsdapp-gcc > CC ice_rxtx.o > CC ice_ethdev.o > cc1: error: unrecognized command line option > '-Wno-address-of-packed-member' [-Werror] > cc1: all warnings being treated as errors > gmake[6]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc/e6c3bc062b1045128114bdf0cfdd236b/dpdk/mk/internal/rte.compile-pre.mk:114: > bnxt_ethdev.o] Error 1 > gmake[5]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc/e6c3bc062b1045128114bdf0cfdd236b/dpdk/mk/rte.subdir.mk:37: > bnxt] Error 2 > gmake[5]: *** Waiting for unfinished jobs.... More than just that *Build Failed Build #1: OS: FreeBSD12-64 Target: x86_64-native-bsdapp-gcc+debug CC ipn3ke_flow.o CC lio_23xx_vf.o cc1: error: unrecognized command line option '-Wno-address-of-packed-member' [-Werror] cc1: all warnings being treated as errors gmake[6]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc+debug/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/internal/rte.compile-pre.mk:116: bnxt_ethdev.o] Error 1 gmake[5]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc+debug/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.subdir.mk:37: bnxt] Error 2 gmake[5]: *** Waiting for unfinished jobs.... CC i40e_flow.o CC rte_eth_null.o -- LD ixgbe_ethdev.o AR librte_pmd_ixgbe.a INSTALL-LIB librte_pmd_ixgbe.a gmake[4]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc+debug/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.subdir.mk:35: net] Error 2 gmake[3]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc+debug/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.sdkbuild.mk:46: drivers] Error 2 gmake[2]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc+debug/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.sdkroot.mk:99: all] Error 2 gmake[1]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc+debug/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.sdkinstall.mk:61: pre_install] Error 2 gmake: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc+debug/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.sdkroot.mk:77: install] Error 2 *Build Failed Build #2: OS: FreeBSD12-64 Target: x86_64-native-bsdapp-gcc+shared CC rte_eth_null.o.pmd.o LD rte_eth_null.o cc1: error: unrecognized command line option '-Wno-address-of-packed-member' [-Werror] cc1: all warnings being treated as errors gmake[6]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc+shared/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/internal/rte.compile-pre.mk:116: bnxt_ethdev.o] Error 1 gmake[6]: *** Waiting for unfinished jobs.... CC ice_rxtx.o == Build drivers/net/ring -- LD ice_ethdev.o CC ice_rxtx_vec_sse.o CC octeontx_pkovf.o gmake[5]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc+shared/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.subdir.mk:35: bnxt] Error 2 gmake[5]: *** Waiting for unfinished jobs.... CC i40e_tm.o CC ice_rxtx_vec_avx2.o -- INSTALL-LIB librte_pmd_ixgbe.so.2.1 LD librte_pmd_qede.so.1.1 INSTALL-LIB librte_pmd_qede.so.1.1 gmake[4]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc+shared/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.subdir.mk:35: net] Error 2 gmake[3]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc+shared/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.sdkbuild.mk:46: drivers] Error 2 gmake[2]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc+shared/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.sdkroot.mk:99: all] Error 2 gmake[1]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc+shared/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.sdkinstall.mk:61: pre_install] Error 2 gmake: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc+shared/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.sdkroot.mk:77: install] Error 2 *Build Failed Build #3: OS: FreeBSD12-64 Target: x86_64-native-bsdapp-gcc CC ice_dcb.o LD rte_eth_null.o cc1: error: unrecognized command line option '-Wno-address-of-packed-member' [-Werror] cc1: all warnings being treated as errors gmake[6]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/internal/rte.compile-pre.mk:114: bnxt_ethdev.o] Error 1 gmake[5]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.subdir.mk:37: bnxt] Error 2 gmake[5]: *** Waiting for unfinished jobs.... CC ixgbe_phy.o CC octeontx_rxtx.o -- LD qede_ethdev.o AR librte_pmd_qede.a INSTALL-LIB librte_pmd_qede.a gmake[4]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.subdir.mk:35: net] Error 2 gmake[3]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.sdkbuild.mk:46: drivers] Error 2 gmake[2]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.sdkroot.mk:99: all] Error 2 gmake[1]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.sdkinstall.mk:61: pre_install] Error 2 gmake: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.sdkroot.mk:77: install] Error 2 *Build Failed Build #4: OS: FreeBSD12-64 Target: x86_64-native-bsdapp-clang #define roundup(x, y) ((((x)+((y)-1))/(y))*(y)) /* to any y */ ^ 1 error generated. gmake[6]: *** [/tmp/FreeBSD12-64_K19.02_Clang6.0.1/x86_64-native-bsdapp-clang/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/internal/rte.compile-pre.mk:114: bnxt_ethdev.o] Error 1 CC ice_flex_pipe.o gmake[5]: *** [/tmp/FreeBSD12-64_K19.02_Clang6.0.1/x86_64-native-bsdapp-clang/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.subdir.mk:35: bnxt] Error 2 gmake[5]: *** Waiting for unfinished jobs.... CC lio_mbox.o AR librte_pmd_null.a -- LD ixgbe_ethdev.o AR librte_pmd_ixgbe.a INSTALL-LIB librte_pmd_ixgbe.a gmake[4]: *** [/tmp/FreeBSD12-64_K19.02_Clang6.0.1/x86_64-native-bsdapp-clang/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.subdir.mk:35: net] Error 2 gmake[3]: *** [/tmp/FreeBSD12-64_K19.02_Clang6.0.1/x86_64-native-bsdapp-clang/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.sdkbuild.mk:46: drivers] Error 2 gmake[2]: *** [/tmp/FreeBSD12-64_K19.02_Clang6.0.1/x86_64-native-bsdapp-clang/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.sdkroot.mk:99: all] Error 2 gmake[1]: *** [/tmp/FreeBSD12-64_K19.02_Clang6.0.1/x86_64-native-bsdapp-clang/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.sdkinstall.mk:61: pre_install] Error 2 gmake: *** [/tmp/FreeBSD12-64_K19.02_Clang6.0.1/x86_64-native-bsdapp-clang/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/rte.sdkroot.mk:77: install] Error 2 *Meson Failed Build #1: OS: UB1604-32 Target:build-gcc-static FAILED: examples/c590b3c@@dpdk-rxtx_callbacks at exe/rxtx_callbacks_main.c.o gcc -Iexamples/c590b3c@@dpdk-rxtx_callbacks at exe -Iexamples -I../examples -Iexamples/rxtx_callbacks -I../examples/rxtx_callbacks -I. -I../ -Iconfig -I../config -Ilib/librte_eal/common/include -I../lib/librte_eal/common/include -I../lib/librte_eal/linux/eal/include -Ilib/librte_eal/common -I../lib/librte_eal/common -Ilib/librte_eal/common/include/arch/x86 -I../lib/librte_eal/common/include/arch/x86 -Ilib/librte_eal -I../lib/librte_eal -Ilib/librte_kvargs -I../lib/librte_kvargs -Ilib/librte_mempool -I../lib/librte_mempool -Ilib/librte_ring -I../lib/librte_ring -Ilib/librte_net -I../lib/librte_net -Ilib/librte_mbuf -I../lib/librte_mbuf -Ilib/librte_ethdev -I../lib/librte_ethdev -Ilib/librte_cmdline -I../lib/librte_cmdline -Ilib/librte_meter -I../lib/librte_meter -fdiagnostics-color=always -pipe -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Werror -O3 -include rte_config.h -Wunused-parameter -Wsign-compare -Wcast-qual -Wno-pointer-to-int-cast -march=native -D_GNU_SOURCE -DALLOW_EXPERIMENTAL_API -MD -MQ 'examples/c590b3c@@dpdk-rxtx_callbacks at exe/rxtx_callbacks_main.c.o' -MF 'examples/c590b3c@@dpdk-rxtx_callbacks at exe/rxtx_callbacks_main.c.o.d' -o 'examples/c590b3c@@dpdk-rxtx_callbacks at exe/rxtx_callbacks_main.c.o' -c ../examples/rxtx_callbacks/main.c ../examples/rxtx_callbacks/main.c: In function ‘port_init’: ../examples/rxtx_callbacks/main.c:172:10: error: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 2 has type ‘uint64_t {aka long long unsigned int}’ [-Werror=format=] printf("TSC Freq ~= %lu\nHW Freq ~= %lu\nRatio : %f\n", ^ ../examples/rxtx_callbacks/main.c:172:10: error: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 3 has type ‘uint64_t {aka long long unsigned int}’ [-Werror=format=] cc1: all warnings being treated as errors [1531/1549] Linking target examples/dpdk-vdpa. [1532/1549] Linking target examples/dpdk-timer. [1533/1549] Linking target examples/dpdk-tep_termination. [1534/1549] Compiling C object 'examples/c590b3c@@dpdk-vhost_crypto at exe/vhost_crypto_main.c.o'. [1535/1549] Compiling C object 'examples/c590b3c@@dpdk-vhost at exe/vhost_main.c.o'. ninja: build stopped: subcommand failed ^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [dpdk-dev] [PATCH v2 0/7] kni: cleanups and improvements 2019-06-11 21:30 ` Stephen Hemminger @ 2019-06-11 21:49 ` Lance Richardson 0 siblings, 0 replies; 63+ messages in thread From: Lance Richardson @ 2019-06-11 21:49 UTC (permalink / raw) To: Stephen Hemminger; +Cc: Ferruh Yigit, dev On Tue, Jun 11, 2019 at 5:30 PM Stephen Hemminger <stephen@networkplumber.org> wrote: > > On Tue, 11 Jun 2019 17:18:42 -0400 > Lance Richardson <lance.richardson@broadcom.com> wrote: > > > On Tue, Jun 11, 2019 at 4:54 PM Stephen Hemminger > > <stephen@networkplumber.org> wrote: > > > > > > On Mon, 10 Jun 2019 10:51:48 -0700 > > > Stephen Hemminger <stephen@networkplumber.org> wrote: > > > > > > > While testing KNI with netvsc, saw lots of places more code > > > > could be safely removed from KNI kernel driver. > > > > > > > > This is still mostly "putting lipstick on a pig" all users > > > > would be better off using virtio_user rather than KNI. > > > > > > > > v2 - get rid of unnecessary padding, combine the unused field patches > > > > > > > > Stephen Hemminger (7): > > > > kni: don't need stubs for rx_mode or ioctl > > > > kni: use netdev_alloc_skb > > > > kni: don't keep stats in kni_net > > > > kni: drop unused fields > > > > kni: use proper type for kni fifo's > > > > kni: return -EFAULT if copy_from_user fails > > > > doc: update KNI documentation > > > > > > > > .../sample_app_ug/kernel_nic_interface.rst | 18 ++--- > > > > kernel/linux/kni/kni_dev.h | 21 ++--- > > > > kernel/linux/kni/kni_misc.c | 17 ++-- > > > > kernel/linux/kni/kni_net.c | 79 +++++-------------- > > > > 4 files changed, 38 insertions(+), 97 deletions(-) > > > > > > > > > > Don't believe patchwork the patch is fine, it is getting falsely blamed > > > for failures caused by other changes in ice, bnxt which fail > > > FreeBSD build. > > > > Do you mean failures like the ones below? If so, I think think they > > might be an unintended > > consequence of commit a385972c3 "mk: disable warning for packed member pointer". > > > > Lance > > > > OS: FreeBSD12-64 > > Target: x86_64-native-bsdapp-gcc > > CC ice_rxtx.o > > CC ice_ethdev.o > > cc1: error: unrecognized command line option > > '-Wno-address-of-packed-member' [-Werror] > > cc1: all warnings being treated as errors > > gmake[6]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc/e6c3bc062b1045128114bdf0cfdd236b/dpdk/mk/internal/rte.compile-pre.mk:114: > > bnxt_ethdev.o] Error 1 > > gmake[5]: *** [/tmp/FreeBSD12-64_K19.02_GCC7.3.0/x86_64-native-bsdapp-gcc/e6c3bc062b1045128114bdf0cfdd236b/dpdk/mk/rte.subdir.mk:37: > > bnxt] Error 2 > > gmake[5]: *** Waiting for unfinished jobs.... > > > More than just that OK, I see now... I'll spin a fix for this one: > OS: FreeBSD12-64 > Target: x86_64-native-bsdapp-clang > #define roundup(x, y) ((((x)+((y)-1))/(y))*(y)) /* to any y */ > ^ > 1 error generated. > gmake[6]: *** [/tmp/FreeBSD12-64_K19.02_Clang6.0.1/x86_64-native-bsdapp-clang/025b54b548ed4ca894cbf5e754039b68/dpdk/mk/internal/rte.compile-pre.mk:114: bnxt_ethdev.o] Error 1 Thanks, Lance ^ permalink raw reply [flat|nested] 63+ messages in thread
* [dpdk-dev] [PATCH v3 0/8] kni: fixes and cleanups 2019-06-10 17:51 [dpdk-dev] [PATCH v2 0/7] kni: cleanups and improvements Stephen Hemminger ` (7 preceding siblings ...) 2019-06-11 20:54 ` [dpdk-dev] [PATCH v2 0/7] kni: cleanups and improvements Stephen Hemminger @ 2019-06-18 16:16 ` Stephen Hemminger 2019-06-18 16:16 ` [dpdk-dev] [PATCH v3 1/8] kni: don't need stubs for rx_mode or ioctl Stephen Hemminger ` (7 more replies) 2019-06-19 18:57 ` [dpdk-dev] [PATCH v3 0/8] kni: cleanups and fixes Stephen Hemminger ` (3 subsequent siblings) 12 siblings, 8 replies; 63+ messages in thread From: Stephen Hemminger @ 2019-06-18 16:16 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger While testing KNI with netvsc, saw lots of places more code could be safely removed from KNI kernel driver. v3 - rebase to current master, add style fix patch v2 - get rid of unnecessary padding, combine the unused field patches Stephen Hemminger (8): kni: don't need stubs for rx_mode or ioctl kni: use netdev_alloc_skb kni: don't keep stats in kni_net kni: drop unused fields kni: use proper type for kni fifo's kni: return -EFAULT if copy_from_user fails doc: update KNI documentation kni: fix style issues .../sample_app_ug/kernel_nic_interface.rst | 18 ++--- kernel/linux/kni/kni_dev.h | 18 ++--- kernel/linux/kni/kni_misc.c | 17 ++-- kernel/linux/kni/kni_net.c | 79 +++++-------------- lib/librte_kni/rte_kni.c | 30 +++---- 5 files changed, 53 insertions(+), 109 deletions(-) -- 2.20.1 ^ permalink raw reply [flat|nested] 63+ messages in thread
* [dpdk-dev] [PATCH v3 1/8] kni: don't need stubs for rx_mode or ioctl 2019-06-18 16:16 ` [dpdk-dev] [PATCH v3 0/8] kni: fixes and cleanups Stephen Hemminger @ 2019-06-18 16:16 ` Stephen Hemminger 2019-06-18 16:16 ` [dpdk-dev] [PATCH v3 2/8] kni: use netdev_alloc_skb Stephen Hemminger ` (6 subsequent siblings) 7 siblings, 0 replies; 63+ messages in thread From: Stephen Hemminger @ 2019-06-18 16:16 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger The netdev subsystem already handles case where network sevice does not support ioctl. If device has no rx_mode hook it is not called. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- kernel/linux/kni/kni_net.c | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c index ad8365877cda..c86337d099ab 100644 --- a/kernel/linux/kni/kni_net.c +++ b/kernel/linux/kni/kni_net.c @@ -593,23 +593,6 @@ kni_net_tx_timeout(struct net_device *dev) netif_wake_queue(dev); } -/* - * Ioctl commands - */ -static int -kni_net_ioctl(struct net_device *dev, struct ifreq *rq, int cmd) -{ - pr_debug("kni_net_ioctl group:%d cmd:%d\n", - ((struct kni_dev *)netdev_priv(dev))->group_id, cmd); - - return -EOPNOTSUPP; -} - -static void -kni_net_set_rx_mode(struct net_device *dev) -{ -} - static int kni_net_change_mtu(struct net_device *dev, int new_mtu) { @@ -758,8 +741,6 @@ static const struct net_device_ops kni_net_netdev_ops = { .ndo_change_rx_flags = kni_net_set_promiscusity, .ndo_start_xmit = kni_net_tx, .ndo_change_mtu = kni_net_change_mtu, - .ndo_do_ioctl = kni_net_ioctl, - .ndo_set_rx_mode = kni_net_set_rx_mode, .ndo_get_stats = kni_net_stats, .ndo_tx_timeout = kni_net_tx_timeout, .ndo_set_mac_address = kni_net_set_mac, -- 2.20.1 ^ permalink raw reply [flat|nested] 63+ messages in thread
* [dpdk-dev] [PATCH v3 2/8] kni: use netdev_alloc_skb 2019-06-18 16:16 ` [dpdk-dev] [PATCH v3 0/8] kni: fixes and cleanups Stephen Hemminger 2019-06-18 16:16 ` [dpdk-dev] [PATCH v3 1/8] kni: don't need stubs for rx_mode or ioctl Stephen Hemminger @ 2019-06-18 16:16 ` Stephen Hemminger 2019-06-18 16:16 ` [dpdk-dev] [PATCH v3 3/8] kni: don't keep stats in kni_net Stephen Hemminger ` (5 subsequent siblings) 7 siblings, 0 replies; 63+ messages in thread From: Stephen Hemminger @ 2019-06-18 16:16 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger netdev_alloc_skb is optimized to any alignment or setup of skb->dev that is required. The kernel has chosen to not pad packets on x86 (for many years), because it is faster. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- kernel/linux/kni/kni_net.c | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c index c86337d099ab..cce5e7eb991f 100644 --- a/kernel/linux/kni/kni_net.c +++ b/kernel/linux/kni/kni_net.c @@ -340,16 +340,13 @@ kni_net_rx_normal(struct kni_dev *kni) data_kva = kva2data_kva(kva); kni->va[i] = pa2va(kni->pa[i], kva); - skb = dev_alloc_skb(len + 2); + skb = netdev_alloc_skb(dev, len); if (!skb) { /* Update statistics */ kni->stats.rx_dropped++; continue; } - /* Align IP on 16B boundary */ - skb_reserve(skb, 2); - if (kva->nb_segs == 1) { memcpy(skb_put(skb, len), data_kva, len); } else { @@ -368,7 +365,6 @@ kni_net_rx_normal(struct kni_dev *kni) } } - skb->dev = dev; skb->protocol = eth_type_trans(skb, dev); skb->ip_summed = CHECKSUM_UNNECESSARY; @@ -512,26 +508,20 @@ kni_net_rx_lo_fifo_skb(struct kni_dev *kni) data_kva = kva2data_kva(kva); kni->va[i] = pa2va(kni->pa[i], kva); - skb = dev_alloc_skb(len + 2); + skb = netdev_alloc_skb(dev, len); if (skb) { - /* Align IP on 16B boundary */ - skb_reserve(skb, 2); memcpy(skb_put(skb, len), data_kva, len); - skb->dev = dev; skb->ip_summed = CHECKSUM_UNNECESSARY; dev_kfree_skb(skb); } /* Simulate real usage, allocate/copy skb twice */ - skb = dev_alloc_skb(len + 2); + skb = netdev_alloc_skb(dev, len); if (skb == NULL) { kni->stats.rx_dropped++; continue; } - /* Align IP on 16B boundary */ - skb_reserve(skb, 2); - if (kva->nb_segs == 1) { memcpy(skb_put(skb, len), data_kva, len); } else { @@ -550,7 +540,6 @@ kni_net_rx_lo_fifo_skb(struct kni_dev *kni) } } - skb->dev = dev; skb->ip_summed = CHECKSUM_UNNECESSARY; kni->stats.rx_bytes += len; -- 2.20.1 ^ permalink raw reply [flat|nested] 63+ messages in thread
* [dpdk-dev] [PATCH v3 3/8] kni: don't keep stats in kni_net 2019-06-18 16:16 ` [dpdk-dev] [PATCH v3 0/8] kni: fixes and cleanups Stephen Hemminger 2019-06-18 16:16 ` [dpdk-dev] [PATCH v3 1/8] kni: don't need stubs for rx_mode or ioctl Stephen Hemminger 2019-06-18 16:16 ` [dpdk-dev] [PATCH v3 2/8] kni: use netdev_alloc_skb Stephen Hemminger @ 2019-06-18 16:16 ` Stephen Hemminger 2019-06-18 16:16 ` [dpdk-dev] [PATCH v3 4/8] kni: drop unused fields Stephen Hemminger ` (4 subsequent siblings) 7 siblings, 0 replies; 63+ messages in thread From: Stephen Hemminger @ 2019-06-18 16:16 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger Since kernel 2.6.28 the network subsystem has provided dev->stats for devices to use statistics handling and is the default if no ndo_get_stats is provided. This allow allows for 64 bit (rather than just 32 bit) statistics with KNI. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- kernel/linux/kni/kni_dev.h | 1 - kernel/linux/kni/kni_net.c | 43 +++++++++++++------------------------- 2 files changed, 15 insertions(+), 29 deletions(-) diff --git a/kernel/linux/kni/kni_dev.h b/kernel/linux/kni/kni_dev.h index d57bce647e4a..21e4b0d92368 100644 --- a/kernel/linux/kni/kni_dev.h +++ b/kernel/linux/kni/kni_dev.h @@ -39,7 +39,6 @@ struct kni_dev { /* kni list */ struct list_head list; - struct net_device_stats stats; int status; uint16_t group_id; /* Group ID of a group of KNI devices */ uint32_t core_id; /* Core ID to bind */ diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c index cce5e7eb991f..06310fec57bb 100644 --- a/kernel/linux/kni/kni_net.c +++ b/kernel/linux/kni/kni_net.c @@ -291,15 +291,15 @@ kni_net_tx(struct sk_buff *skb, struct net_device *dev) /* Free skb and update statistics */ dev_kfree_skb(skb); - kni->stats.tx_bytes += len; - kni->stats.tx_packets++; + dev->stats.tx_bytes += len; + dev->stats.tx_packets++; return NETDEV_TX_OK; drop: /* Free skb and update statistics */ dev_kfree_skb(skb); - kni->stats.tx_dropped++; + dev->stats.tx_dropped++; return NETDEV_TX_OK; } @@ -343,7 +343,7 @@ kni_net_rx_normal(struct kni_dev *kni) skb = netdev_alloc_skb(dev, len); if (!skb) { /* Update statistics */ - kni->stats.rx_dropped++; + dev->stats.rx_dropped++; continue; } @@ -372,8 +372,8 @@ kni_net_rx_normal(struct kni_dev *kni) netif_rx_ni(skb); /* Update statistics */ - kni->stats.rx_bytes += len; - kni->stats.rx_packets++; + dev->stats.rx_bytes += len; + dev->stats.rx_packets++; } /* Burst enqueue mbufs into free_q */ @@ -396,6 +396,7 @@ kni_net_rx_lo_fifo(struct kni_dev *kni) void *data_kva; struct rte_kni_mbuf *alloc_kva; void *alloc_data_kva; + struct net_device *dev = kni->net_dev; /* Get the number of entries in rx_q */ num_rq = kni_fifo_count(kni->rx_q); @@ -443,8 +444,8 @@ kni_net_rx_lo_fifo(struct kni_dev *kni) alloc_kva->pkt_len = len; alloc_kva->data_len = len; - kni->stats.tx_bytes += len; - kni->stats.rx_bytes += len; + dev->stats.tx_bytes += len; + dev->stats.rx_bytes += len; } /* Burst enqueue mbufs into tx_q */ @@ -464,8 +465,8 @@ kni_net_rx_lo_fifo(struct kni_dev *kni) * Update statistic, and enqueue/dequeue failure is impossible, * as all queues are checked at first. */ - kni->stats.tx_packets += num; - kni->stats.rx_packets += num; + dev->stats.tx_packets += num; + dev->stats.rx_packets += num; } /* @@ -518,7 +519,7 @@ kni_net_rx_lo_fifo_skb(struct kni_dev *kni) /* Simulate real usage, allocate/copy skb twice */ skb = netdev_alloc_skb(dev, len); if (skb == NULL) { - kni->stats.rx_dropped++; + dev->stats.rx_dropped++; continue; } @@ -542,8 +543,8 @@ kni_net_rx_lo_fifo_skb(struct kni_dev *kni) skb->ip_summed = CHECKSUM_UNNECESSARY; - kni->stats.rx_bytes += len; - kni->stats.rx_packets++; + dev->stats.rx_bytes += len; + dev->stats.rx_packets++; /* call tx interface */ kni_net_tx(skb, dev); @@ -573,12 +574,10 @@ kni_net_rx(struct kni_dev *kni) static void kni_net_tx_timeout(struct net_device *dev) { - struct kni_dev *kni = netdev_priv(dev); - pr_debug("Transmit timeout at %ld, latency %ld\n", jiffies, jiffies - dev_trans_start(dev)); - kni->stats.tx_errors++; + dev->stats.tx_errors++; netif_wake_queue(dev); } @@ -627,17 +626,6 @@ kni_net_poll_resp(struct kni_dev *kni) wake_up_interruptible(&kni->wq); } -/* - * Return statistics to the caller - */ -static struct net_device_stats * -kni_net_stats(struct net_device *dev) -{ - struct kni_dev *kni = netdev_priv(dev); - - return &kni->stats; -} - /* * Fill the eth header */ @@ -730,7 +718,6 @@ static const struct net_device_ops kni_net_netdev_ops = { .ndo_change_rx_flags = kni_net_set_promiscusity, .ndo_start_xmit = kni_net_tx, .ndo_change_mtu = kni_net_change_mtu, - .ndo_get_stats = kni_net_stats, .ndo_tx_timeout = kni_net_tx_timeout, .ndo_set_mac_address = kni_net_set_mac, #ifdef HAVE_CHANGE_CARRIER_CB -- 2.20.1 ^ permalink raw reply [flat|nested] 63+ messages in thread
* [dpdk-dev] [PATCH v3 4/8] kni: drop unused fields 2019-06-18 16:16 ` [dpdk-dev] [PATCH v3 0/8] kni: fixes and cleanups Stephen Hemminger ` (2 preceding siblings ...) 2019-06-18 16:16 ` [dpdk-dev] [PATCH v3 3/8] kni: don't keep stats in kni_net Stephen Hemminger @ 2019-06-18 16:16 ` Stephen Hemminger 2019-06-18 16:16 ` [dpdk-dev] [PATCH v3 5/8] kni: use proper type for kni fifo's Stephen Hemminger ` (3 subsequent siblings) 7 siblings, 0 replies; 63+ messages in thread From: Stephen Hemminger @ 2019-06-18 16:16 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger The kni net structure only exists in driver no API/ABI. Several fields were either totally unused or set and never used. The fields in dev_info do need to stay in the ABI but kernel can ignore them. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> kni: drop unused status element Yet another ethtool leftover. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- kernel/linux/kni/kni_dev.h | 5 ----- kernel/linux/kni/kni_misc.c | 1 - 2 files changed, 6 deletions(-) diff --git a/kernel/linux/kni/kni_dev.h b/kernel/linux/kni/kni_dev.h index 21e4b0d92368..f3e6c3ca4efa 100644 --- a/kernel/linux/kni/kni_dev.h +++ b/kernel/linux/kni/kni_dev.h @@ -39,8 +39,6 @@ struct kni_dev { /* kni list */ struct list_head list; - int status; - uint16_t group_id; /* Group ID of a group of KNI devices */ uint32_t core_id; /* Core ID to bind */ char name[RTE_KNI_NAMESIZE]; /* Network device name */ struct task_struct *pthread; @@ -79,9 +77,6 @@ struct kni_dev { /* mbuf size */ uint32_t mbuf_size; - /* synchro for request processing */ - unsigned long synchro; - /* buffers */ void *pa[MBUF_BURST_SZ]; void *va[MBUF_BURST_SZ]; diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c index 1fc5eeb9c8ca..b59cf24c2184 100644 --- a/kernel/linux/kni/kni_misc.c +++ b/kernel/linux/kni/kni_misc.c @@ -346,7 +346,6 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num, kni = netdev_priv(net_dev); kni->net_dev = net_dev; - kni->group_id = dev_info.group_id; kni->core_id = dev_info.core_id; strncpy(kni->name, dev_info.name, RTE_KNI_NAMESIZE); -- 2.20.1 ^ permalink raw reply [flat|nested] 63+ messages in thread
* [dpdk-dev] [PATCH v3 5/8] kni: use proper type for kni fifo's 2019-06-18 16:16 ` [dpdk-dev] [PATCH v3 0/8] kni: fixes and cleanups Stephen Hemminger ` (3 preceding siblings ...) 2019-06-18 16:16 ` [dpdk-dev] [PATCH v3 4/8] kni: drop unused fields Stephen Hemminger @ 2019-06-18 16:16 ` Stephen Hemminger 2019-06-18 16:16 ` [dpdk-dev] [PATCH v3 6/8] kni: return -EFAULT if copy_from_user fails Stephen Hemminger ` (2 subsequent siblings) 7 siblings, 0 replies; 63+ messages in thread From: Stephen Hemminger @ 2019-06-18 16:16 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger Using void * instead of proper type is unsafe practice. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- kernel/linux/kni/kni_dev.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/kernel/linux/kni/kni_dev.h b/kernel/linux/kni/kni_dev.h index f3e6c3ca4efa..ceba5f73c1d9 100644 --- a/kernel/linux/kni/kni_dev.h +++ b/kernel/linux/kni/kni_dev.h @@ -51,22 +51,22 @@ struct kni_dev { struct net_device *net_dev; /* queue for packets to be sent out */ - void *tx_q; + struct rte_kni_fifo *tx_q; /* queue for the packets received */ - void *rx_q; + struct rte_kni_fifo *rx_q; /* queue for the allocated mbufs those can be used to save sk buffs */ - void *alloc_q; + struct rte_kni_fifo *alloc_q; /* free queue for the mbufs to be freed */ - void *free_q; + struct rte_kni_fifo *free_q; /* request queue */ - void *req_q; + struct rte_kni_fifo *req_q; /* response queue */ - void *resp_q; + struct rte_kni_fifo *resp_q; void *sync_kva; void *sync_va; -- 2.20.1 ^ permalink raw reply [flat|nested] 63+ messages in thread
* [dpdk-dev] [PATCH v3 6/8] kni: return -EFAULT if copy_from_user fails 2019-06-18 16:16 ` [dpdk-dev] [PATCH v3 0/8] kni: fixes and cleanups Stephen Hemminger ` (4 preceding siblings ...) 2019-06-18 16:16 ` [dpdk-dev] [PATCH v3 5/8] kni: use proper type for kni fifo's Stephen Hemminger @ 2019-06-18 16:16 ` Stephen Hemminger 2019-06-18 16:16 ` [dpdk-dev] [PATCH v3 7/8] doc: update KNI documentation Stephen Hemminger 2019-06-18 16:16 ` [dpdk-dev] [PATCH v3 8/8] kni: fix style issues Stephen Hemminger 7 siblings, 0 replies; 63+ messages in thread From: Stephen Hemminger @ 2019-06-18 16:16 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger The correct thing to return if user gives a bad data is to return -EFAULT. Logging is also discouraged because it could be used as a DoS attack. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- kernel/linux/kni/kni_misc.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c index b59cf24c2184..be45f823408f 100644 --- a/kernel/linux/kni/kni_misc.c +++ b/kernel/linux/kni/kni_misc.c @@ -301,11 +301,8 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num, return -EINVAL; /* Copy kni info from user space */ - ret = copy_from_user(&dev_info, (void *)ioctl_param, sizeof(dev_info)); - if (ret) { - pr_err("copy_from_user in kni_ioctl_create"); - return -EIO; - } + if (copy_from_user(&dev_info, (void *)ioctl_param, sizeof(dev_info))) + return -EFAULT; /* Check if name is zero-ended */ if (strnlen(dev_info.name, sizeof(dev_info.name)) == sizeof(dev_info.name)) { @@ -427,15 +424,12 @@ kni_ioctl_release(struct net *net, uint32_t ioctl_num, if (_IOC_SIZE(ioctl_num) > sizeof(dev_info)) return -EINVAL; - ret = copy_from_user(&dev_info, (void *)ioctl_param, sizeof(dev_info)); - if (ret) { - pr_err("copy_from_user in kni_ioctl_release"); - return -EIO; - } + if (copy_from_user(&dev_info, (void *)ioctl_param, sizeof(dev_info))) + return -EFAULT; /* Release the network device according to its name */ if (strlen(dev_info.name) == 0) - return ret; + return -EINVAL; down_write(&knet->kni_list_lock); list_for_each_entry_safe(dev, n, &knet->kni_list_head, list) { -- 2.20.1 ^ permalink raw reply [flat|nested] 63+ messages in thread
* [dpdk-dev] [PATCH v3 7/8] doc: update KNI documentation 2019-06-18 16:16 ` [dpdk-dev] [PATCH v3 0/8] kni: fixes and cleanups Stephen Hemminger ` (5 preceding siblings ...) 2019-06-18 16:16 ` [dpdk-dev] [PATCH v3 6/8] kni: return -EFAULT if copy_from_user fails Stephen Hemminger @ 2019-06-18 16:16 ` Stephen Hemminger 2019-06-18 16:16 ` [dpdk-dev] [PATCH v3 8/8] kni: fix style issues Stephen Hemminger 7 siblings, 0 replies; 63+ messages in thread From: Stephen Hemminger @ 2019-06-18 16:16 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger Make the KNI documentation reflect modern kernel networking. Ifconfig has been superseded by iproute2 for 15 years. Iproute2 is well maintained, supports current feature set. Ethtool is no longer supported by KNI. Tshark is a better replacement for tcpdump. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- .../sample_app_ug/kernel_nic_interface.rst | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/doc/guides/sample_app_ug/kernel_nic_interface.rst b/doc/guides/sample_app_ug/kernel_nic_interface.rst index a7e549d5213a..422bd8c98465 100644 --- a/doc/guides/sample_app_ug/kernel_nic_interface.rst +++ b/doc/guides/sample_app_ug/kernel_nic_interface.rst @@ -21,14 +21,14 @@ The FIFO queues contain pointers to data packets in the DPDK. This: * Provides a faster mechanism to interface with the kernel net stack and eliminates system calls -* Facilitates the DPDK using standard Linux* userspace net tools (tcpdump, ftp, and so on) +* Facilitates the DPDK using standard Linux* userspace net tools (tshark, rsync, and so on) * Eliminate the copy_to_user and copy_from_user operations on packets. The Kernel NIC Interface sample application is a simple example that demonstrates the use of the DPDK to create a path for packets to go through the Linux* kernel. This is done by creating one or more kernel net devices for each of the DPDK ports. -The application allows the use of standard Linux tools (ethtool, ifconfig, tcpdump) with the DPDK ports and +The application allows the use of standard Linux tools (iproute, tshark) with the DPDK ports and also the exchange of packets between the DPDK application and the Linux* kernel. The Kernel NIC Interface sample application requires that the @@ -220,13 +220,13 @@ Enable KNI interface and assign an IP address: .. code-block:: console - # ifconfig vEth0_0 192.168.0.1 + # ip addr add dev vEth0_0 192.168.0.1 Show KNI interface configuration and statistics: .. code-block:: console - # ifconfig vEth0_0 + # ip -s -d addr show vEth0_0 The user can also check and reset the packet statistics inside the ``kni`` application by sending the app the USR1 and USR2 signals: @@ -234,16 +234,16 @@ application by sending the app the USR1 and USR2 signals: .. code-block:: console # Print statistics - # kill -SIGUSR1 `pidof kni` + # pkill -USR1 kni # Zero statistics - # kill -SIGUSR2 `pidof kni` + # pkill -USR2 kni Dump network traffic: .. code-block:: console - # tcpdump -i vEth0_0 + # tshark -n -i vEth0_0 The normal Linux commands can also be used to change the MAC address and MTU size used by the physical NIC which corresponds to the KNI interface. @@ -254,13 +254,13 @@ Change the MAC address: .. code-block:: console - # ifconfig vEth0_0 hw ether 0C:01:02:03:04:08 + # ip link set dev vEth0_0 lladdr 0C:01:02:03:04:08 Change the MTU size: .. code-block:: console - # ifconfig vEth0_0 mtu 1450 + # ip link set dev vEth0_0 mtu 1450 When the ``kni`` application is closed, all the KNI interfaces are deleted from the Linux kernel. -- 2.20.1 ^ permalink raw reply [flat|nested] 63+ messages in thread
* [dpdk-dev] [PATCH v3 8/8] kni: fix style issues 2019-06-18 16:16 ` [dpdk-dev] [PATCH v3 0/8] kni: fixes and cleanups Stephen Hemminger ` (6 preceding siblings ...) 2019-06-18 16:16 ` [dpdk-dev] [PATCH v3 7/8] doc: update KNI documentation Stephen Hemminger @ 2019-06-18 16:16 ` Stephen Hemminger 7 siblings, 0 replies; 63+ messages in thread From: Stephen Hemminger @ 2019-06-18 16:16 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger rte_kni does not follow standard style rules. Noticed some extra \ line continuation etc. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- lib/librte_kni/rte_kni.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/lib/librte_kni/rte_kni.c b/lib/librte_kni/rte_kni.c index e29d0cc7df3c..4828df3ca1e6 100644 --- a/lib/librte_kni/rte_kni.c +++ b/lib/librte_kni/rte_kni.c @@ -59,7 +59,7 @@ struct rte_kni { uint16_t group_id; /**< Group ID of KNI devices */ uint32_t slot_id; /**< KNI pool slot ID */ struct rte_mempool *pktmbuf_pool; /**< pkt mbuf mempool */ - unsigned mbuf_size; /**< mbuf size */ + unsigned int mbuf_size; /**< mbuf size */ const struct rte_memzone *m_tx_q; /**< TX queue memzone */ const struct rte_memzone *m_rx_q; /**< RX queue memzone */ @@ -78,7 +78,7 @@ struct rte_kni { /* For request & response */ struct rte_kni_fifo *req_q; /**< Request queue */ struct rte_kni_fifo *resp_q; /**< Response queue */ - void * sync_addr; /**< Req/Resp Mem address */ + void *sync_addr; /**< Req/Resp Mem address */ struct rte_kni_ops ops; /**< operations for request */ }; @@ -473,7 +473,7 @@ kni_config_promiscusity(uint16_t port_id, uint8_t to_on) int rte_kni_handle_request(struct rte_kni *kni) { - unsigned ret; + unsigned int ret; struct rte_kni_request *req = NULL; if (kni == NULL) @@ -498,8 +498,8 @@ rte_kni_handle_request(struct rte_kni *kni) break; case RTE_KNI_REQ_CFG_NETWORK_IF: /* Set network interface up/down */ if (kni->ops.config_network_if) - req->result = kni->ops.config_network_if(\ - kni->ops.port_id, req->if_up); + req->result = kni->ops.config_network_if(kni->ops.port_id, + req->if_up); break; case RTE_KNI_REQ_CHANGE_MAC_ADDR: /* Change MAC Address */ if (kni->ops.config_mac_address) @@ -534,7 +534,7 @@ rte_kni_handle_request(struct rte_kni *kni) } unsigned -rte_kni_tx_burst(struct rte_kni *kni, struct rte_mbuf **mbufs, unsigned num) +rte_kni_tx_burst(struct rte_kni *kni, struct rte_mbuf **mbufs, unsigned int num) { void *phy_mbufs[num]; unsigned int ret; @@ -552,9 +552,9 @@ rte_kni_tx_burst(struct rte_kni *kni, struct rte_mbuf **mbufs, unsigned num) } unsigned -rte_kni_rx_burst(struct rte_kni *kni, struct rte_mbuf **mbufs, unsigned num) +rte_kni_rx_burst(struct rte_kni *kni, struct rte_mbuf **mbufs, unsigned int num) { - unsigned ret = kni_fifo_get(kni->tx_q, (void **)mbufs, num); + unsigned int ret = kni_fifo_get(kni->tx_q, (void **)mbufs, num); /* If buffers removed, allocate mbufs and then put them into alloc_q */ if (ret) @@ -605,7 +605,7 @@ kni_allocate_mbufs(struct rte_kni *kni) return; } - allocq_free = (kni->alloc_q->read - kni->alloc_q->write - 1) \ + allocq_free = (kni->alloc_q->read - kni->alloc_q->write - 1) & (MAX_MBUF_BURST_NUM - 1); for (i = 0; i < allocq_free; i++) { pkts[i] = rte_pktmbuf_alloc(kni->pktmbuf_pool); @@ -659,7 +659,7 @@ static enum kni_ops_status kni_check_request_register(struct rte_kni_ops *ops) { /* check if KNI request ops has been registered*/ - if( NULL == ops ) + if (NULL == ops) return KNI_REQ_NO_REGISTER; if ((ops->change_mtu == NULL) @@ -672,22 +672,22 @@ kni_check_request_register(struct rte_kni_ops *ops) } int -rte_kni_register_handlers(struct rte_kni *kni,struct rte_kni_ops *ops) +rte_kni_register_handlers(struct rte_kni *kni, struct rte_kni_ops *ops) { enum kni_ops_status req_status; - if (NULL == ops) { + if (ops == NULL) { RTE_LOG(ERR, KNI, "Invalid KNI request operation.\n"); return -1; } - if (NULL == kni) { + if (kni == NULL) { RTE_LOG(ERR, KNI, "Invalid kni info.\n"); return -1; } req_status = kni_check_request_register(&kni->ops); - if ( KNI_REQ_REGISTERED == req_status) { + if (req_status == KNI_REQ_REGISTERED) { RTE_LOG(ERR, KNI, "The KNI request operation has already registered.\n"); return -1; } @@ -699,7 +699,7 @@ rte_kni_register_handlers(struct rte_kni *kni,struct rte_kni_ops *ops) int rte_kni_unregister_handlers(struct rte_kni *kni) { - if (NULL == kni) { + if (kni == NULL) { RTE_LOG(ERR, KNI, "Invalid kni info.\n"); return -1; } -- 2.20.1 ^ permalink raw reply [flat|nested] 63+ messages in thread
* [dpdk-dev] [PATCH v3 0/8] kni: cleanups and fixes 2019-06-10 17:51 [dpdk-dev] [PATCH v2 0/7] kni: cleanups and improvements Stephen Hemminger ` (8 preceding siblings ...) 2019-06-18 16:16 ` [dpdk-dev] [PATCH v3 0/8] kni: fixes and cleanups Stephen Hemminger @ 2019-06-19 18:57 ` Stephen Hemminger 2019-06-19 18:57 ` [dpdk-dev] [PATCH v3 1/8] kni: don't need stubs for rx_mode or ioctl Stephen Hemminger ` (7 more replies) 2019-06-19 18:59 ` [dpdk-dev] [PATCH v4 0/8] kni: fixes and cleanups Stephen Hemminger ` (2 subsequent siblings) 12 siblings, 8 replies; 63+ messages in thread From: Stephen Hemminger @ 2019-06-19 18:57 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger While testing KNI with netvsc, saw lots of places more code could be safely removed from KNI kernel driver. This is still mostly "putting lipstick on a pig" all users would be better off using virtio_user rather than KNI. v3 - more style cleanups in last patch v2 - get rid of unnecessary padding, combine the unused field patches Stephen Hemminger (8): kni: don't need stubs for rx_mode or ioctl kni: use netdev_alloc_skb kni: don't keep stats in kni_net kni: drop unused fields kni: use proper type for kni fifo's kni: return -EFAULT if copy_from_user fails doc: update KNI documentation kni: fix style issues .../sample_app_ug/kernel_nic_interface.rst | 18 ++--- kernel/linux/kni/kni_dev.h | 18 ++--- kernel/linux/kni/kni_misc.c | 17 ++-- kernel/linux/kni/kni_net.c | 79 +++++-------------- lib/librte_kni/rte_kni.c | 38 ++++----- 5 files changed, 57 insertions(+), 113 deletions(-) -- 2.20.1 ^ permalink raw reply [flat|nested] 63+ messages in thread
* [dpdk-dev] [PATCH v3 1/8] kni: don't need stubs for rx_mode or ioctl 2019-06-19 18:57 ` [dpdk-dev] [PATCH v3 0/8] kni: cleanups and fixes Stephen Hemminger @ 2019-06-19 18:57 ` Stephen Hemminger 2019-06-19 18:57 ` [dpdk-dev] [PATCH v3 2/8] kni: use netdev_alloc_skb Stephen Hemminger ` (6 subsequent siblings) 7 siblings, 0 replies; 63+ messages in thread From: Stephen Hemminger @ 2019-06-19 18:57 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger, Stephen Hemminger From: Stephen Hemminger <sthemmin@microsoft.com> The netdev subsystem already handles case where network sevice does not support ioctl. If device has no rx_mode hook it is not called. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- kernel/linux/kni/kni_net.c | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c index ad8365877cda..c86337d099ab 100644 --- a/kernel/linux/kni/kni_net.c +++ b/kernel/linux/kni/kni_net.c @@ -593,23 +593,6 @@ kni_net_tx_timeout(struct net_device *dev) netif_wake_queue(dev); } -/* - * Ioctl commands - */ -static int -kni_net_ioctl(struct net_device *dev, struct ifreq *rq, int cmd) -{ - pr_debug("kni_net_ioctl group:%d cmd:%d\n", - ((struct kni_dev *)netdev_priv(dev))->group_id, cmd); - - return -EOPNOTSUPP; -} - -static void -kni_net_set_rx_mode(struct net_device *dev) -{ -} - static int kni_net_change_mtu(struct net_device *dev, int new_mtu) { @@ -758,8 +741,6 @@ static const struct net_device_ops kni_net_netdev_ops = { .ndo_change_rx_flags = kni_net_set_promiscusity, .ndo_start_xmit = kni_net_tx, .ndo_change_mtu = kni_net_change_mtu, - .ndo_do_ioctl = kni_net_ioctl, - .ndo_set_rx_mode = kni_net_set_rx_mode, .ndo_get_stats = kni_net_stats, .ndo_tx_timeout = kni_net_tx_timeout, .ndo_set_mac_address = kni_net_set_mac, -- 2.20.1 ^ permalink raw reply [flat|nested] 63+ messages in thread
* [dpdk-dev] [PATCH v3 2/8] kni: use netdev_alloc_skb 2019-06-19 18:57 ` [dpdk-dev] [PATCH v3 0/8] kni: cleanups and fixes Stephen Hemminger 2019-06-19 18:57 ` [dpdk-dev] [PATCH v3 1/8] kni: don't need stubs for rx_mode or ioctl Stephen Hemminger @ 2019-06-19 18:57 ` Stephen Hemminger 2019-06-19 18:57 ` [dpdk-dev] [PATCH v3 3/8] kni: don't keep stats in kni_net Stephen Hemminger ` (5 subsequent siblings) 7 siblings, 0 replies; 63+ messages in thread From: Stephen Hemminger @ 2019-06-19 18:57 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger, Stephen Hemminger From: Stephen Hemminger <sthemmin@microsoft.com> netdev_alloc_skb is optimized to any alignment or setup of skb->dev that is required. The kernel has chosen to not pad packets on x86 (for many years), because it is faster. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- kernel/linux/kni/kni_net.c | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c index c86337d099ab..cce5e7eb991f 100644 --- a/kernel/linux/kni/kni_net.c +++ b/kernel/linux/kni/kni_net.c @@ -340,16 +340,13 @@ kni_net_rx_normal(struct kni_dev *kni) data_kva = kva2data_kva(kva); kni->va[i] = pa2va(kni->pa[i], kva); - skb = dev_alloc_skb(len + 2); + skb = netdev_alloc_skb(dev, len); if (!skb) { /* Update statistics */ kni->stats.rx_dropped++; continue; } - /* Align IP on 16B boundary */ - skb_reserve(skb, 2); - if (kva->nb_segs == 1) { memcpy(skb_put(skb, len), data_kva, len); } else { @@ -368,7 +365,6 @@ kni_net_rx_normal(struct kni_dev *kni) } } - skb->dev = dev; skb->protocol = eth_type_trans(skb, dev); skb->ip_summed = CHECKSUM_UNNECESSARY; @@ -512,26 +508,20 @@ kni_net_rx_lo_fifo_skb(struct kni_dev *kni) data_kva = kva2data_kva(kva); kni->va[i] = pa2va(kni->pa[i], kva); - skb = dev_alloc_skb(len + 2); + skb = netdev_alloc_skb(dev, len); if (skb) { - /* Align IP on 16B boundary */ - skb_reserve(skb, 2); memcpy(skb_put(skb, len), data_kva, len); - skb->dev = dev; skb->ip_summed = CHECKSUM_UNNECESSARY; dev_kfree_skb(skb); } /* Simulate real usage, allocate/copy skb twice */ - skb = dev_alloc_skb(len + 2); + skb = netdev_alloc_skb(dev, len); if (skb == NULL) { kni->stats.rx_dropped++; continue; } - /* Align IP on 16B boundary */ - skb_reserve(skb, 2); - if (kva->nb_segs == 1) { memcpy(skb_put(skb, len), data_kva, len); } else { @@ -550,7 +540,6 @@ kni_net_rx_lo_fifo_skb(struct kni_dev *kni) } } - skb->dev = dev; skb->ip_summed = CHECKSUM_UNNECESSARY; kni->stats.rx_bytes += len; -- 2.20.1 ^ permalink raw reply [flat|nested] 63+ messages in thread
* [dpdk-dev] [PATCH v3 3/8] kni: don't keep stats in kni_net 2019-06-19 18:57 ` [dpdk-dev] [PATCH v3 0/8] kni: cleanups and fixes Stephen Hemminger 2019-06-19 18:57 ` [dpdk-dev] [PATCH v3 1/8] kni: don't need stubs for rx_mode or ioctl Stephen Hemminger 2019-06-19 18:57 ` [dpdk-dev] [PATCH v3 2/8] kni: use netdev_alloc_skb Stephen Hemminger @ 2019-06-19 18:57 ` Stephen Hemminger 2019-06-19 18:57 ` [dpdk-dev] [PATCH v3 4/8] kni: drop unused fields Stephen Hemminger ` (4 subsequent siblings) 7 siblings, 0 replies; 63+ messages in thread From: Stephen Hemminger @ 2019-06-19 18:57 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger, Stephen Hemminger From: Stephen Hemminger <sthemmin@microsoft.com> Since kernel 2.6.28 the network subsystem has provided dev->stats for devices to use statistics handling and is the default if no ndo_get_stats is provided. This allow allows for 64 bit (rather than just 32 bit) statistics with KNI. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- kernel/linux/kni/kni_dev.h | 1 - kernel/linux/kni/kni_net.c | 43 +++++++++++++------------------------- 2 files changed, 15 insertions(+), 29 deletions(-) diff --git a/kernel/linux/kni/kni_dev.h b/kernel/linux/kni/kni_dev.h index d57bce647e4a..21e4b0d92368 100644 --- a/kernel/linux/kni/kni_dev.h +++ b/kernel/linux/kni/kni_dev.h @@ -39,7 +39,6 @@ struct kni_dev { /* kni list */ struct list_head list; - struct net_device_stats stats; int status; uint16_t group_id; /* Group ID of a group of KNI devices */ uint32_t core_id; /* Core ID to bind */ diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c index cce5e7eb991f..06310fec57bb 100644 --- a/kernel/linux/kni/kni_net.c +++ b/kernel/linux/kni/kni_net.c @@ -291,15 +291,15 @@ kni_net_tx(struct sk_buff *skb, struct net_device *dev) /* Free skb and update statistics */ dev_kfree_skb(skb); - kni->stats.tx_bytes += len; - kni->stats.tx_packets++; + dev->stats.tx_bytes += len; + dev->stats.tx_packets++; return NETDEV_TX_OK; drop: /* Free skb and update statistics */ dev_kfree_skb(skb); - kni->stats.tx_dropped++; + dev->stats.tx_dropped++; return NETDEV_TX_OK; } @@ -343,7 +343,7 @@ kni_net_rx_normal(struct kni_dev *kni) skb = netdev_alloc_skb(dev, len); if (!skb) { /* Update statistics */ - kni->stats.rx_dropped++; + dev->stats.rx_dropped++; continue; } @@ -372,8 +372,8 @@ kni_net_rx_normal(struct kni_dev *kni) netif_rx_ni(skb); /* Update statistics */ - kni->stats.rx_bytes += len; - kni->stats.rx_packets++; + dev->stats.rx_bytes += len; + dev->stats.rx_packets++; } /* Burst enqueue mbufs into free_q */ @@ -396,6 +396,7 @@ kni_net_rx_lo_fifo(struct kni_dev *kni) void *data_kva; struct rte_kni_mbuf *alloc_kva; void *alloc_data_kva; + struct net_device *dev = kni->net_dev; /* Get the number of entries in rx_q */ num_rq = kni_fifo_count(kni->rx_q); @@ -443,8 +444,8 @@ kni_net_rx_lo_fifo(struct kni_dev *kni) alloc_kva->pkt_len = len; alloc_kva->data_len = len; - kni->stats.tx_bytes += len; - kni->stats.rx_bytes += len; + dev->stats.tx_bytes += len; + dev->stats.rx_bytes += len; } /* Burst enqueue mbufs into tx_q */ @@ -464,8 +465,8 @@ kni_net_rx_lo_fifo(struct kni_dev *kni) * Update statistic, and enqueue/dequeue failure is impossible, * as all queues are checked at first. */ - kni->stats.tx_packets += num; - kni->stats.rx_packets += num; + dev->stats.tx_packets += num; + dev->stats.rx_packets += num; } /* @@ -518,7 +519,7 @@ kni_net_rx_lo_fifo_skb(struct kni_dev *kni) /* Simulate real usage, allocate/copy skb twice */ skb = netdev_alloc_skb(dev, len); if (skb == NULL) { - kni->stats.rx_dropped++; + dev->stats.rx_dropped++; continue; } @@ -542,8 +543,8 @@ kni_net_rx_lo_fifo_skb(struct kni_dev *kni) skb->ip_summed = CHECKSUM_UNNECESSARY; - kni->stats.rx_bytes += len; - kni->stats.rx_packets++; + dev->stats.rx_bytes += len; + dev->stats.rx_packets++; /* call tx interface */ kni_net_tx(skb, dev); @@ -573,12 +574,10 @@ kni_net_rx(struct kni_dev *kni) static void kni_net_tx_timeout(struct net_device *dev) { - struct kni_dev *kni = netdev_priv(dev); - pr_debug("Transmit timeout at %ld, latency %ld\n", jiffies, jiffies - dev_trans_start(dev)); - kni->stats.tx_errors++; + dev->stats.tx_errors++; netif_wake_queue(dev); } @@ -627,17 +626,6 @@ kni_net_poll_resp(struct kni_dev *kni) wake_up_interruptible(&kni->wq); } -/* - * Return statistics to the caller - */ -static struct net_device_stats * -kni_net_stats(struct net_device *dev) -{ - struct kni_dev *kni = netdev_priv(dev); - - return &kni->stats; -} - /* * Fill the eth header */ @@ -730,7 +718,6 @@ static const struct net_device_ops kni_net_netdev_ops = { .ndo_change_rx_flags = kni_net_set_promiscusity, .ndo_start_xmit = kni_net_tx, .ndo_change_mtu = kni_net_change_mtu, - .ndo_get_stats = kni_net_stats, .ndo_tx_timeout = kni_net_tx_timeout, .ndo_set_mac_address = kni_net_set_mac, #ifdef HAVE_CHANGE_CARRIER_CB -- 2.20.1 ^ permalink raw reply [flat|nested] 63+ messages in thread
* [dpdk-dev] [PATCH v3 4/8] kni: drop unused fields 2019-06-19 18:57 ` [dpdk-dev] [PATCH v3 0/8] kni: cleanups and fixes Stephen Hemminger ` (2 preceding siblings ...) 2019-06-19 18:57 ` [dpdk-dev] [PATCH v3 3/8] kni: don't keep stats in kni_net Stephen Hemminger @ 2019-06-19 18:57 ` Stephen Hemminger 2019-06-19 18:57 ` [dpdk-dev] [PATCH v3 5/8] kni: use proper type for kni fifo's Stephen Hemminger ` (3 subsequent siblings) 7 siblings, 0 replies; 63+ messages in thread From: Stephen Hemminger @ 2019-06-19 18:57 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger, Stephen Hemminger From: Stephen Hemminger <sthemmin@microsoft.com> The kni net structure only exists in driver no API/ABI. Several fields were either totally unused or set and never used. The fields in dev_info do need to stay in the ABI but kernel can ignore them. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> kni: drop unused status element Yet another ethtool leftover. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- kernel/linux/kni/kni_dev.h | 5 ----- kernel/linux/kni/kni_misc.c | 1 - 2 files changed, 6 deletions(-) diff --git a/kernel/linux/kni/kni_dev.h b/kernel/linux/kni/kni_dev.h index 21e4b0d92368..f3e6c3ca4efa 100644 --- a/kernel/linux/kni/kni_dev.h +++ b/kernel/linux/kni/kni_dev.h @@ -39,8 +39,6 @@ struct kni_dev { /* kni list */ struct list_head list; - int status; - uint16_t group_id; /* Group ID of a group of KNI devices */ uint32_t core_id; /* Core ID to bind */ char name[RTE_KNI_NAMESIZE]; /* Network device name */ struct task_struct *pthread; @@ -79,9 +77,6 @@ struct kni_dev { /* mbuf size */ uint32_t mbuf_size; - /* synchro for request processing */ - unsigned long synchro; - /* buffers */ void *pa[MBUF_BURST_SZ]; void *va[MBUF_BURST_SZ]; diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c index 1fc5eeb9c8ca..b59cf24c2184 100644 --- a/kernel/linux/kni/kni_misc.c +++ b/kernel/linux/kni/kni_misc.c @@ -346,7 +346,6 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num, kni = netdev_priv(net_dev); kni->net_dev = net_dev; - kni->group_id = dev_info.group_id; kni->core_id = dev_info.core_id; strncpy(kni->name, dev_info.name, RTE_KNI_NAMESIZE); -- 2.20.1 ^ permalink raw reply [flat|nested] 63+ messages in thread
* [dpdk-dev] [PATCH v3 5/8] kni: use proper type for kni fifo's 2019-06-19 18:57 ` [dpdk-dev] [PATCH v3 0/8] kni: cleanups and fixes Stephen Hemminger ` (3 preceding siblings ...) 2019-06-19 18:57 ` [dpdk-dev] [PATCH v3 4/8] kni: drop unused fields Stephen Hemminger @ 2019-06-19 18:57 ` Stephen Hemminger 2019-06-19 18:57 ` [dpdk-dev] [PATCH v3 6/8] kni: return -EFAULT if copy_from_user fails Stephen Hemminger ` (2 subsequent siblings) 7 siblings, 0 replies; 63+ messages in thread From: Stephen Hemminger @ 2019-06-19 18:57 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger, Stephen Hemminger From: Stephen Hemminger <sthemmin@microsoft.com> Using void * instead of proper type is unsafe practice. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- kernel/linux/kni/kni_dev.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/kernel/linux/kni/kni_dev.h b/kernel/linux/kni/kni_dev.h index f3e6c3ca4efa..ceba5f73c1d9 100644 --- a/kernel/linux/kni/kni_dev.h +++ b/kernel/linux/kni/kni_dev.h @@ -51,22 +51,22 @@ struct kni_dev { struct net_device *net_dev; /* queue for packets to be sent out */ - void *tx_q; + struct rte_kni_fifo *tx_q; /* queue for the packets received */ - void *rx_q; + struct rte_kni_fifo *rx_q; /* queue for the allocated mbufs those can be used to save sk buffs */ - void *alloc_q; + struct rte_kni_fifo *alloc_q; /* free queue for the mbufs to be freed */ - void *free_q; + struct rte_kni_fifo *free_q; /* request queue */ - void *req_q; + struct rte_kni_fifo *req_q; /* response queue */ - void *resp_q; + struct rte_kni_fifo *resp_q; void *sync_kva; void *sync_va; -- 2.20.1 ^ permalink raw reply [flat|nested] 63+ messages in thread
* [dpdk-dev] [PATCH v3 6/8] kni: return -EFAULT if copy_from_user fails 2019-06-19 18:57 ` [dpdk-dev] [PATCH v3 0/8] kni: cleanups and fixes Stephen Hemminger ` (4 preceding siblings ...) 2019-06-19 18:57 ` [dpdk-dev] [PATCH v3 5/8] kni: use proper type for kni fifo's Stephen Hemminger @ 2019-06-19 18:57 ` Stephen Hemminger 2019-06-19 18:57 ` [dpdk-dev] [PATCH v3 7/8] doc: update KNI documentation Stephen Hemminger 2019-06-19 18:57 ` [dpdk-dev] [PATCH v3 8/8] kni: fix style issues Stephen Hemminger 7 siblings, 0 replies; 63+ messages in thread From: Stephen Hemminger @ 2019-06-19 18:57 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger, Stephen Hemminger From: Stephen Hemminger <sthemmin@microsoft.com> The correct thing to return if user gives a bad data is to return -EFAULT. Logging is also discouraged because it could be used as a DoS attack. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- kernel/linux/kni/kni_misc.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c index b59cf24c2184..be45f823408f 100644 --- a/kernel/linux/kni/kni_misc.c +++ b/kernel/linux/kni/kni_misc.c @@ -301,11 +301,8 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num, return -EINVAL; /* Copy kni info from user space */ - ret = copy_from_user(&dev_info, (void *)ioctl_param, sizeof(dev_info)); - if (ret) { - pr_err("copy_from_user in kni_ioctl_create"); - return -EIO; - } + if (copy_from_user(&dev_info, (void *)ioctl_param, sizeof(dev_info))) + return -EFAULT; /* Check if name is zero-ended */ if (strnlen(dev_info.name, sizeof(dev_info.name)) == sizeof(dev_info.name)) { @@ -427,15 +424,12 @@ kni_ioctl_release(struct net *net, uint32_t ioctl_num, if (_IOC_SIZE(ioctl_num) > sizeof(dev_info)) return -EINVAL; - ret = copy_from_user(&dev_info, (void *)ioctl_param, sizeof(dev_info)); - if (ret) { - pr_err("copy_from_user in kni_ioctl_release"); - return -EIO; - } + if (copy_from_user(&dev_info, (void *)ioctl_param, sizeof(dev_info))) + return -EFAULT; /* Release the network device according to its name */ if (strlen(dev_info.name) == 0) - return ret; + return -EINVAL; down_write(&knet->kni_list_lock); list_for_each_entry_safe(dev, n, &knet->kni_list_head, list) { -- 2.20.1 ^ permalink raw reply [flat|nested] 63+ messages in thread
* [dpdk-dev] [PATCH v3 7/8] doc: update KNI documentation 2019-06-19 18:57 ` [dpdk-dev] [PATCH v3 0/8] kni: cleanups and fixes Stephen Hemminger ` (5 preceding siblings ...) 2019-06-19 18:57 ` [dpdk-dev] [PATCH v3 6/8] kni: return -EFAULT if copy_from_user fails Stephen Hemminger @ 2019-06-19 18:57 ` Stephen Hemminger 2019-06-19 18:57 ` [dpdk-dev] [PATCH v3 8/8] kni: fix style issues Stephen Hemminger 7 siblings, 0 replies; 63+ messages in thread From: Stephen Hemminger @ 2019-06-19 18:57 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger, Stephen Hemminger From: Stephen Hemminger <sthemmin@microsoft.com> Make the KNI documentation reflect modern kernel networking. Ifconfig has been superseded by iproute2 for 15 years. Iproute2 is well maintained, supports current feature set. Ethtool is no longer supported by KNI. Tshark is a better replacement for tcpdump. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- .../sample_app_ug/kernel_nic_interface.rst | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/doc/guides/sample_app_ug/kernel_nic_interface.rst b/doc/guides/sample_app_ug/kernel_nic_interface.rst index a7e549d5213a..422bd8c98465 100644 --- a/doc/guides/sample_app_ug/kernel_nic_interface.rst +++ b/doc/guides/sample_app_ug/kernel_nic_interface.rst @@ -21,14 +21,14 @@ The FIFO queues contain pointers to data packets in the DPDK. This: * Provides a faster mechanism to interface with the kernel net stack and eliminates system calls -* Facilitates the DPDK using standard Linux* userspace net tools (tcpdump, ftp, and so on) +* Facilitates the DPDK using standard Linux* userspace net tools (tshark, rsync, and so on) * Eliminate the copy_to_user and copy_from_user operations on packets. The Kernel NIC Interface sample application is a simple example that demonstrates the use of the DPDK to create a path for packets to go through the Linux* kernel. This is done by creating one or more kernel net devices for each of the DPDK ports. -The application allows the use of standard Linux tools (ethtool, ifconfig, tcpdump) with the DPDK ports and +The application allows the use of standard Linux tools (iproute, tshark) with the DPDK ports and also the exchange of packets between the DPDK application and the Linux* kernel. The Kernel NIC Interface sample application requires that the @@ -220,13 +220,13 @@ Enable KNI interface and assign an IP address: .. code-block:: console - # ifconfig vEth0_0 192.168.0.1 + # ip addr add dev vEth0_0 192.168.0.1 Show KNI interface configuration and statistics: .. code-block:: console - # ifconfig vEth0_0 + # ip -s -d addr show vEth0_0 The user can also check and reset the packet statistics inside the ``kni`` application by sending the app the USR1 and USR2 signals: @@ -234,16 +234,16 @@ application by sending the app the USR1 and USR2 signals: .. code-block:: console # Print statistics - # kill -SIGUSR1 `pidof kni` + # pkill -USR1 kni # Zero statistics - # kill -SIGUSR2 `pidof kni` + # pkill -USR2 kni Dump network traffic: .. code-block:: console - # tcpdump -i vEth0_0 + # tshark -n -i vEth0_0 The normal Linux commands can also be used to change the MAC address and MTU size used by the physical NIC which corresponds to the KNI interface. @@ -254,13 +254,13 @@ Change the MAC address: .. code-block:: console - # ifconfig vEth0_0 hw ether 0C:01:02:03:04:08 + # ip link set dev vEth0_0 lladdr 0C:01:02:03:04:08 Change the MTU size: .. code-block:: console - # ifconfig vEth0_0 mtu 1450 + # ip link set dev vEth0_0 mtu 1450 When the ``kni`` application is closed, all the KNI interfaces are deleted from the Linux kernel. -- 2.20.1 ^ permalink raw reply [flat|nested] 63+ messages in thread
* [dpdk-dev] [PATCH v3 8/8] kni: fix style issues 2019-06-19 18:57 ` [dpdk-dev] [PATCH v3 0/8] kni: cleanups and fixes Stephen Hemminger ` (6 preceding siblings ...) 2019-06-19 18:57 ` [dpdk-dev] [PATCH v3 7/8] doc: update KNI documentation Stephen Hemminger @ 2019-06-19 18:57 ` Stephen Hemminger 7 siblings, 0 replies; 63+ messages in thread From: Stephen Hemminger @ 2019-06-19 18:57 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger, Stephen Hemminger From: Stephen Hemminger <sthemmin@microsoft.com> rte_kni does not follow standard style rules. Noticed some extra \ line continuation etc. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- lib/librte_kni/rte_kni.c | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/lib/librte_kni/rte_kni.c b/lib/librte_kni/rte_kni.c index e29d0cc7df3c..9b6acc382fc3 100644 --- a/lib/librte_kni/rte_kni.c +++ b/lib/librte_kni/rte_kni.c @@ -59,7 +59,7 @@ struct rte_kni { uint16_t group_id; /**< Group ID of KNI devices */ uint32_t slot_id; /**< KNI pool slot ID */ struct rte_mempool *pktmbuf_pool; /**< pkt mbuf mempool */ - unsigned mbuf_size; /**< mbuf size */ + unsigned int mbuf_size; /**< mbuf size */ const struct rte_memzone *m_tx_q; /**< TX queue memzone */ const struct rte_memzone *m_rx_q; /**< RX queue memzone */ @@ -78,7 +78,7 @@ struct rte_kni { /* For request & response */ struct rte_kni_fifo *req_q; /**< Request queue */ struct rte_kni_fifo *resp_q; /**< Response queue */ - void * sync_addr; /**< Req/Resp Mem address */ + void *sync_addr; /**< Req/Resp Mem address */ struct rte_kni_ops ops; /**< operations for request */ }; @@ -473,7 +473,7 @@ kni_config_promiscusity(uint16_t port_id, uint8_t to_on) int rte_kni_handle_request(struct rte_kni *kni) { - unsigned ret; + unsigned int ret; struct rte_kni_request *req = NULL; if (kni == NULL) @@ -498,8 +498,8 @@ rte_kni_handle_request(struct rte_kni *kni) break; case RTE_KNI_REQ_CFG_NETWORK_IF: /* Set network interface up/down */ if (kni->ops.config_network_if) - req->result = kni->ops.config_network_if(\ - kni->ops.port_id, req->if_up); + req->result = kni->ops.config_network_if(kni->ops.port_id, + req->if_up); break; case RTE_KNI_REQ_CHANGE_MAC_ADDR: /* Change MAC Address */ if (kni->ops.config_mac_address) @@ -534,7 +534,7 @@ rte_kni_handle_request(struct rte_kni *kni) } unsigned -rte_kni_tx_burst(struct rte_kni *kni, struct rte_mbuf **mbufs, unsigned num) +rte_kni_tx_burst(struct rte_kni *kni, struct rte_mbuf **mbufs, unsigned int num) { void *phy_mbufs[num]; unsigned int ret; @@ -552,9 +552,9 @@ rte_kni_tx_burst(struct rte_kni *kni, struct rte_mbuf **mbufs, unsigned num) } unsigned -rte_kni_rx_burst(struct rte_kni *kni, struct rte_mbuf **mbufs, unsigned num) +rte_kni_rx_burst(struct rte_kni *kni, struct rte_mbuf **mbufs, unsigned int num) { - unsigned ret = kni_fifo_get(kni->tx_q, (void **)mbufs, num); + unsigned int ret = kni_fifo_get(kni->tx_q, (void **)mbufs, num); /* If buffers removed, allocate mbufs and then put them into alloc_q */ if (ret) @@ -605,7 +605,7 @@ kni_allocate_mbufs(struct rte_kni *kni) return; } - allocq_free = (kni->alloc_q->read - kni->alloc_q->write - 1) \ + allocq_free = (kni->alloc_q->read - kni->alloc_q->write - 1) & (MAX_MBUF_BURST_NUM - 1); for (i = 0; i < allocq_free; i++) { pkts[i] = rte_pktmbuf_alloc(kni->pktmbuf_pool); @@ -659,35 +659,35 @@ static enum kni_ops_status kni_check_request_register(struct rte_kni_ops *ops) { /* check if KNI request ops has been registered*/ - if( NULL == ops ) + if (ops == NULL) return KNI_REQ_NO_REGISTER; - if ((ops->change_mtu == NULL) - && (ops->config_network_if == NULL) - && (ops->config_mac_address == NULL) - && (ops->config_promiscusity == NULL)) + if (ops->change_mtu == NULL + && ops->config_network_if == NULL + && ops->config_mac_address == NULL + && ops->config_promiscusity == NULL) return KNI_REQ_NO_REGISTER; return KNI_REQ_REGISTERED; } int -rte_kni_register_handlers(struct rte_kni *kni,struct rte_kni_ops *ops) +rte_kni_register_handlers(struct rte_kni *kni, struct rte_kni_ops *ops) { enum kni_ops_status req_status; - if (NULL == ops) { + if (ops == NULL) { RTE_LOG(ERR, KNI, "Invalid KNI request operation.\n"); return -1; } - if (NULL == kni) { + if (kni == NULL) { RTE_LOG(ERR, KNI, "Invalid kni info.\n"); return -1; } req_status = kni_check_request_register(&kni->ops); - if ( KNI_REQ_REGISTERED == req_status) { + if (req_status == KNI_REQ_REGISTERED) { RTE_LOG(ERR, KNI, "The KNI request operation has already registered.\n"); return -1; } @@ -699,7 +699,7 @@ rte_kni_register_handlers(struct rte_kni *kni,struct rte_kni_ops *ops) int rte_kni_unregister_handlers(struct rte_kni *kni) { - if (NULL == kni) { + if (kni == NULL) { RTE_LOG(ERR, KNI, "Invalid kni info.\n"); return -1; } -- 2.20.1 ^ permalink raw reply [flat|nested] 63+ messages in thread
* [dpdk-dev] [PATCH v4 0/8] kni: fixes and cleanups 2019-06-10 17:51 [dpdk-dev] [PATCH v2 0/7] kni: cleanups and improvements Stephen Hemminger ` (9 preceding siblings ...) 2019-06-19 18:57 ` [dpdk-dev] [PATCH v3 0/8] kni: cleanups and fixes Stephen Hemminger @ 2019-06-19 18:59 ` Stephen Hemminger 2019-06-19 18:59 ` [dpdk-dev] [PATCH v4 1/8] kni: don't need stubs for rx_mode or ioctl Stephen Hemminger ` (7 more replies) 2019-06-20 19:20 ` [dpdk-dev] [PATCH v5 0/9] kni: fixes and cleanups Stephen Hemminger 2019-06-24 16:47 ` [dpdk-dev] [PATCH v6 0/9] kni: fixes and cleanups Stephen Hemminger 12 siblings, 8 replies; 63+ messages in thread From: Stephen Hemminger @ 2019-06-19 18:59 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger While testing KNI with netvsc, saw lots of places more code could be safely removed from KNI kernel driver. v4 - add more style fixes v3 - rebase to current master, add style fix patch v2 - get rid of unnecessary padding, combine the unused field patches Stephen Hemminger (8): kni: don't need stubs for rx_mode or ioctl kni: use netdev_alloc_skb kni: don't keep stats in kni_net kni: drop unused fields kni: use proper type for kni fifo's kni: return -EFAULT if copy_from_user fails doc: update KNI documentation kni: fix style issues .../sample_app_ug/kernel_nic_interface.rst | 18 ++--- kernel/linux/kni/kni_dev.h | 18 ++--- kernel/linux/kni/kni_misc.c | 17 ++-- kernel/linux/kni/kni_net.c | 79 +++++-------------- lib/librte_kni/rte_kni.c | 38 ++++----- 5 files changed, 57 insertions(+), 113 deletions(-) -- 2.20.1 ^ permalink raw reply [flat|nested] 63+ messages in thread
* [dpdk-dev] [PATCH v4 1/8] kni: don't need stubs for rx_mode or ioctl 2019-06-19 18:59 ` [dpdk-dev] [PATCH v4 0/8] kni: fixes and cleanups Stephen Hemminger @ 2019-06-19 18:59 ` Stephen Hemminger 2019-06-19 18:59 ` [dpdk-dev] [PATCH v4 2/8] kni: use netdev_alloc_skb Stephen Hemminger ` (6 subsequent siblings) 7 siblings, 0 replies; 63+ messages in thread From: Stephen Hemminger @ 2019-06-19 18:59 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger, Stephen Hemminger From: Stephen Hemminger <sthemmin@microsoft.com> The netdev subsystem already handles case where network sevice does not support ioctl. If device has no rx_mode hook it is not called. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- kernel/linux/kni/kni_net.c | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c index ad8365877cda..c86337d099ab 100644 --- a/kernel/linux/kni/kni_net.c +++ b/kernel/linux/kni/kni_net.c @@ -593,23 +593,6 @@ kni_net_tx_timeout(struct net_device *dev) netif_wake_queue(dev); } -/* - * Ioctl commands - */ -static int -kni_net_ioctl(struct net_device *dev, struct ifreq *rq, int cmd) -{ - pr_debug("kni_net_ioctl group:%d cmd:%d\n", - ((struct kni_dev *)netdev_priv(dev))->group_id, cmd); - - return -EOPNOTSUPP; -} - -static void -kni_net_set_rx_mode(struct net_device *dev) -{ -} - static int kni_net_change_mtu(struct net_device *dev, int new_mtu) { @@ -758,8 +741,6 @@ static const struct net_device_ops kni_net_netdev_ops = { .ndo_change_rx_flags = kni_net_set_promiscusity, .ndo_start_xmit = kni_net_tx, .ndo_change_mtu = kni_net_change_mtu, - .ndo_do_ioctl = kni_net_ioctl, - .ndo_set_rx_mode = kni_net_set_rx_mode, .ndo_get_stats = kni_net_stats, .ndo_tx_timeout = kni_net_tx_timeout, .ndo_set_mac_address = kni_net_set_mac, -- 2.20.1 ^ permalink raw reply [flat|nested] 63+ messages in thread
* [dpdk-dev] [PATCH v4 2/8] kni: use netdev_alloc_skb 2019-06-19 18:59 ` [dpdk-dev] [PATCH v4 0/8] kni: fixes and cleanups Stephen Hemminger 2019-06-19 18:59 ` [dpdk-dev] [PATCH v4 1/8] kni: don't need stubs for rx_mode or ioctl Stephen Hemminger @ 2019-06-19 18:59 ` Stephen Hemminger 2019-06-19 18:59 ` [dpdk-dev] [PATCH v4 3/8] kni: don't keep stats in kni_net Stephen Hemminger ` (5 subsequent siblings) 7 siblings, 0 replies; 63+ messages in thread From: Stephen Hemminger @ 2019-06-19 18:59 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger, Stephen Hemminger From: Stephen Hemminger <sthemmin@microsoft.com> netdev_alloc_skb is optimized to any alignment or setup of skb->dev that is required. The kernel has chosen to not pad packets on x86 (for many years), because it is faster. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- kernel/linux/kni/kni_net.c | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c index c86337d099ab..cce5e7eb991f 100644 --- a/kernel/linux/kni/kni_net.c +++ b/kernel/linux/kni/kni_net.c @@ -340,16 +340,13 @@ kni_net_rx_normal(struct kni_dev *kni) data_kva = kva2data_kva(kva); kni->va[i] = pa2va(kni->pa[i], kva); - skb = dev_alloc_skb(len + 2); + skb = netdev_alloc_skb(dev, len); if (!skb) { /* Update statistics */ kni->stats.rx_dropped++; continue; } - /* Align IP on 16B boundary */ - skb_reserve(skb, 2); - if (kva->nb_segs == 1) { memcpy(skb_put(skb, len), data_kva, len); } else { @@ -368,7 +365,6 @@ kni_net_rx_normal(struct kni_dev *kni) } } - skb->dev = dev; skb->protocol = eth_type_trans(skb, dev); skb->ip_summed = CHECKSUM_UNNECESSARY; @@ -512,26 +508,20 @@ kni_net_rx_lo_fifo_skb(struct kni_dev *kni) data_kva = kva2data_kva(kva); kni->va[i] = pa2va(kni->pa[i], kva); - skb = dev_alloc_skb(len + 2); + skb = netdev_alloc_skb(dev, len); if (skb) { - /* Align IP on 16B boundary */ - skb_reserve(skb, 2); memcpy(skb_put(skb, len), data_kva, len); - skb->dev = dev; skb->ip_summed = CHECKSUM_UNNECESSARY; dev_kfree_skb(skb); } /* Simulate real usage, allocate/copy skb twice */ - skb = dev_alloc_skb(len + 2); + skb = netdev_alloc_skb(dev, len); if (skb == NULL) { kni->stats.rx_dropped++; continue; } - /* Align IP on 16B boundary */ - skb_reserve(skb, 2); - if (kva->nb_segs == 1) { memcpy(skb_put(skb, len), data_kva, len); } else { @@ -550,7 +540,6 @@ kni_net_rx_lo_fifo_skb(struct kni_dev *kni) } } - skb->dev = dev; skb->ip_summed = CHECKSUM_UNNECESSARY; kni->stats.rx_bytes += len; -- 2.20.1 ^ permalink raw reply [flat|nested] 63+ messages in thread
* [dpdk-dev] [PATCH v4 3/8] kni: don't keep stats in kni_net 2019-06-19 18:59 ` [dpdk-dev] [PATCH v4 0/8] kni: fixes and cleanups Stephen Hemminger 2019-06-19 18:59 ` [dpdk-dev] [PATCH v4 1/8] kni: don't need stubs for rx_mode or ioctl Stephen Hemminger 2019-06-19 18:59 ` [dpdk-dev] [PATCH v4 2/8] kni: use netdev_alloc_skb Stephen Hemminger @ 2019-06-19 18:59 ` Stephen Hemminger 2019-06-19 18:59 ` [dpdk-dev] [PATCH v4 4/8] kni: drop unused fields Stephen Hemminger ` (4 subsequent siblings) 7 siblings, 0 replies; 63+ messages in thread From: Stephen Hemminger @ 2019-06-19 18:59 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger, Stephen Hemminger From: Stephen Hemminger <sthemmin@microsoft.com> Since kernel 2.6.28 the network subsystem has provided dev->stats for devices to use statistics handling and is the default if no ndo_get_stats is provided. This allow allows for 64 bit (rather than just 32 bit) statistics with KNI. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- kernel/linux/kni/kni_dev.h | 1 - kernel/linux/kni/kni_net.c | 43 +++++++++++++------------------------- 2 files changed, 15 insertions(+), 29 deletions(-) diff --git a/kernel/linux/kni/kni_dev.h b/kernel/linux/kni/kni_dev.h index d57bce647e4a..21e4b0d92368 100644 --- a/kernel/linux/kni/kni_dev.h +++ b/kernel/linux/kni/kni_dev.h @@ -39,7 +39,6 @@ struct kni_dev { /* kni list */ struct list_head list; - struct net_device_stats stats; int status; uint16_t group_id; /* Group ID of a group of KNI devices */ uint32_t core_id; /* Core ID to bind */ diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c index cce5e7eb991f..06310fec57bb 100644 --- a/kernel/linux/kni/kni_net.c +++ b/kernel/linux/kni/kni_net.c @@ -291,15 +291,15 @@ kni_net_tx(struct sk_buff *skb, struct net_device *dev) /* Free skb and update statistics */ dev_kfree_skb(skb); - kni->stats.tx_bytes += len; - kni->stats.tx_packets++; + dev->stats.tx_bytes += len; + dev->stats.tx_packets++; return NETDEV_TX_OK; drop: /* Free skb and update statistics */ dev_kfree_skb(skb); - kni->stats.tx_dropped++; + dev->stats.tx_dropped++; return NETDEV_TX_OK; } @@ -343,7 +343,7 @@ kni_net_rx_normal(struct kni_dev *kni) skb = netdev_alloc_skb(dev, len); if (!skb) { /* Update statistics */ - kni->stats.rx_dropped++; + dev->stats.rx_dropped++; continue; } @@ -372,8 +372,8 @@ kni_net_rx_normal(struct kni_dev *kni) netif_rx_ni(skb); /* Update statistics */ - kni->stats.rx_bytes += len; - kni->stats.rx_packets++; + dev->stats.rx_bytes += len; + dev->stats.rx_packets++; } /* Burst enqueue mbufs into free_q */ @@ -396,6 +396,7 @@ kni_net_rx_lo_fifo(struct kni_dev *kni) void *data_kva; struct rte_kni_mbuf *alloc_kva; void *alloc_data_kva; + struct net_device *dev = kni->net_dev; /* Get the number of entries in rx_q */ num_rq = kni_fifo_count(kni->rx_q); @@ -443,8 +444,8 @@ kni_net_rx_lo_fifo(struct kni_dev *kni) alloc_kva->pkt_len = len; alloc_kva->data_len = len; - kni->stats.tx_bytes += len; - kni->stats.rx_bytes += len; + dev->stats.tx_bytes += len; + dev->stats.rx_bytes += len; } /* Burst enqueue mbufs into tx_q */ @@ -464,8 +465,8 @@ kni_net_rx_lo_fifo(struct kni_dev *kni) * Update statistic, and enqueue/dequeue failure is impossible, * as all queues are checked at first. */ - kni->stats.tx_packets += num; - kni->stats.rx_packets += num; + dev->stats.tx_packets += num; + dev->stats.rx_packets += num; } /* @@ -518,7 +519,7 @@ kni_net_rx_lo_fifo_skb(struct kni_dev *kni) /* Simulate real usage, allocate/copy skb twice */ skb = netdev_alloc_skb(dev, len); if (skb == NULL) { - kni->stats.rx_dropped++; + dev->stats.rx_dropped++; continue; } @@ -542,8 +543,8 @@ kni_net_rx_lo_fifo_skb(struct kni_dev *kni) skb->ip_summed = CHECKSUM_UNNECESSARY; - kni->stats.rx_bytes += len; - kni->stats.rx_packets++; + dev->stats.rx_bytes += len; + dev->stats.rx_packets++; /* call tx interface */ kni_net_tx(skb, dev); @@ -573,12 +574,10 @@ kni_net_rx(struct kni_dev *kni) static void kni_net_tx_timeout(struct net_device *dev) { - struct kni_dev *kni = netdev_priv(dev); - pr_debug("Transmit timeout at %ld, latency %ld\n", jiffies, jiffies - dev_trans_start(dev)); - kni->stats.tx_errors++; + dev->stats.tx_errors++; netif_wake_queue(dev); } @@ -627,17 +626,6 @@ kni_net_poll_resp(struct kni_dev *kni) wake_up_interruptible(&kni->wq); } -/* - * Return statistics to the caller - */ -static struct net_device_stats * -kni_net_stats(struct net_device *dev) -{ - struct kni_dev *kni = netdev_priv(dev); - - return &kni->stats; -} - /* * Fill the eth header */ @@ -730,7 +718,6 @@ static const struct net_device_ops kni_net_netdev_ops = { .ndo_change_rx_flags = kni_net_set_promiscusity, .ndo_start_xmit = kni_net_tx, .ndo_change_mtu = kni_net_change_mtu, - .ndo_get_stats = kni_net_stats, .ndo_tx_timeout = kni_net_tx_timeout, .ndo_set_mac_address = kni_net_set_mac, #ifdef HAVE_CHANGE_CARRIER_CB -- 2.20.1 ^ permalink raw reply [flat|nested] 63+ messages in thread
* [dpdk-dev] [PATCH v4 4/8] kni: drop unused fields 2019-06-19 18:59 ` [dpdk-dev] [PATCH v4 0/8] kni: fixes and cleanups Stephen Hemminger ` (2 preceding siblings ...) 2019-06-19 18:59 ` [dpdk-dev] [PATCH v4 3/8] kni: don't keep stats in kni_net Stephen Hemminger @ 2019-06-19 18:59 ` Stephen Hemminger 2019-06-19 18:59 ` [dpdk-dev] [PATCH v4 5/8] kni: use proper type for kni fifo's Stephen Hemminger ` (3 subsequent siblings) 7 siblings, 0 replies; 63+ messages in thread From: Stephen Hemminger @ 2019-06-19 18:59 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger, Stephen Hemminger From: Stephen Hemminger <sthemmin@microsoft.com> The kni net structure only exists in driver no API/ABI. Several fields were either totally unused or set and never used. The fields in dev_info do need to stay in the ABI but kernel can ignore them. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> kni: drop unused status element Yet another ethtool leftover. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- kernel/linux/kni/kni_dev.h | 5 ----- kernel/linux/kni/kni_misc.c | 1 - 2 files changed, 6 deletions(-) diff --git a/kernel/linux/kni/kni_dev.h b/kernel/linux/kni/kni_dev.h index 21e4b0d92368..f3e6c3ca4efa 100644 --- a/kernel/linux/kni/kni_dev.h +++ b/kernel/linux/kni/kni_dev.h @@ -39,8 +39,6 @@ struct kni_dev { /* kni list */ struct list_head list; - int status; - uint16_t group_id; /* Group ID of a group of KNI devices */ uint32_t core_id; /* Core ID to bind */ char name[RTE_KNI_NAMESIZE]; /* Network device name */ struct task_struct *pthread; @@ -79,9 +77,6 @@ struct kni_dev { /* mbuf size */ uint32_t mbuf_size; - /* synchro for request processing */ - unsigned long synchro; - /* buffers */ void *pa[MBUF_BURST_SZ]; void *va[MBUF_BURST_SZ]; diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c index 1fc5eeb9c8ca..b59cf24c2184 100644 --- a/kernel/linux/kni/kni_misc.c +++ b/kernel/linux/kni/kni_misc.c @@ -346,7 +346,6 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num, kni = netdev_priv(net_dev); kni->net_dev = net_dev; - kni->group_id = dev_info.group_id; kni->core_id = dev_info.core_id; strncpy(kni->name, dev_info.name, RTE_KNI_NAMESIZE); -- 2.20.1 ^ permalink raw reply [flat|nested] 63+ messages in thread
* [dpdk-dev] [PATCH v4 5/8] kni: use proper type for kni fifo's 2019-06-19 18:59 ` [dpdk-dev] [PATCH v4 0/8] kni: fixes and cleanups Stephen Hemminger ` (3 preceding siblings ...) 2019-06-19 18:59 ` [dpdk-dev] [PATCH v4 4/8] kni: drop unused fields Stephen Hemminger @ 2019-06-19 18:59 ` Stephen Hemminger 2019-06-19 18:59 ` [dpdk-dev] [PATCH v4 6/8] kni: return -EFAULT if copy_from_user fails Stephen Hemminger ` (2 subsequent siblings) 7 siblings, 0 replies; 63+ messages in thread From: Stephen Hemminger @ 2019-06-19 18:59 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger, Stephen Hemminger From: Stephen Hemminger <sthemmin@microsoft.com> Using void * instead of proper type is unsafe practice. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- kernel/linux/kni/kni_dev.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/kernel/linux/kni/kni_dev.h b/kernel/linux/kni/kni_dev.h index f3e6c3ca4efa..ceba5f73c1d9 100644 --- a/kernel/linux/kni/kni_dev.h +++ b/kernel/linux/kni/kni_dev.h @@ -51,22 +51,22 @@ struct kni_dev { struct net_device *net_dev; /* queue for packets to be sent out */ - void *tx_q; + struct rte_kni_fifo *tx_q; /* queue for the packets received */ - void *rx_q; + struct rte_kni_fifo *rx_q; /* queue for the allocated mbufs those can be used to save sk buffs */ - void *alloc_q; + struct rte_kni_fifo *alloc_q; /* free queue for the mbufs to be freed */ - void *free_q; + struct rte_kni_fifo *free_q; /* request queue */ - void *req_q; + struct rte_kni_fifo *req_q; /* response queue */ - void *resp_q; + struct rte_kni_fifo *resp_q; void *sync_kva; void *sync_va; -- 2.20.1 ^ permalink raw reply [flat|nested] 63+ messages in thread
* [dpdk-dev] [PATCH v4 6/8] kni: return -EFAULT if copy_from_user fails 2019-06-19 18:59 ` [dpdk-dev] [PATCH v4 0/8] kni: fixes and cleanups Stephen Hemminger ` (4 preceding siblings ...) 2019-06-19 18:59 ` [dpdk-dev] [PATCH v4 5/8] kni: use proper type for kni fifo's Stephen Hemminger @ 2019-06-19 18:59 ` Stephen Hemminger 2019-06-19 18:59 ` [dpdk-dev] [PATCH v4 7/8] doc: update KNI documentation Stephen Hemminger 2019-06-19 18:59 ` [dpdk-dev] [PATCH v4 8/8] kni: fix style issues Stephen Hemminger 7 siblings, 0 replies; 63+ messages in thread From: Stephen Hemminger @ 2019-06-19 18:59 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger, Stephen Hemminger From: Stephen Hemminger <sthemmin@microsoft.com> The correct thing to return if user gives a bad data is to return -EFAULT. Logging is also discouraged because it could be used as a DoS attack. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- kernel/linux/kni/kni_misc.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c index b59cf24c2184..be45f823408f 100644 --- a/kernel/linux/kni/kni_misc.c +++ b/kernel/linux/kni/kni_misc.c @@ -301,11 +301,8 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num, return -EINVAL; /* Copy kni info from user space */ - ret = copy_from_user(&dev_info, (void *)ioctl_param, sizeof(dev_info)); - if (ret) { - pr_err("copy_from_user in kni_ioctl_create"); - return -EIO; - } + if (copy_from_user(&dev_info, (void *)ioctl_param, sizeof(dev_info))) + return -EFAULT; /* Check if name is zero-ended */ if (strnlen(dev_info.name, sizeof(dev_info.name)) == sizeof(dev_info.name)) { @@ -427,15 +424,12 @@ kni_ioctl_release(struct net *net, uint32_t ioctl_num, if (_IOC_SIZE(ioctl_num) > sizeof(dev_info)) return -EINVAL; - ret = copy_from_user(&dev_info, (void *)ioctl_param, sizeof(dev_info)); - if (ret) { - pr_err("copy_from_user in kni_ioctl_release"); - return -EIO; - } + if (copy_from_user(&dev_info, (void *)ioctl_param, sizeof(dev_info))) + return -EFAULT; /* Release the network device according to its name */ if (strlen(dev_info.name) == 0) - return ret; + return -EINVAL; down_write(&knet->kni_list_lock); list_for_each_entry_safe(dev, n, &knet->kni_list_head, list) { -- 2.20.1 ^ permalink raw reply [flat|nested] 63+ messages in thread
* [dpdk-dev] [PATCH v4 7/8] doc: update KNI documentation 2019-06-19 18:59 ` [dpdk-dev] [PATCH v4 0/8] kni: fixes and cleanups Stephen Hemminger ` (5 preceding siblings ...) 2019-06-19 18:59 ` [dpdk-dev] [PATCH v4 6/8] kni: return -EFAULT if copy_from_user fails Stephen Hemminger @ 2019-06-19 18:59 ` Stephen Hemminger 2019-06-19 18:59 ` [dpdk-dev] [PATCH v4 8/8] kni: fix style issues Stephen Hemminger 7 siblings, 0 replies; 63+ messages in thread From: Stephen Hemminger @ 2019-06-19 18:59 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger, Stephen Hemminger From: Stephen Hemminger <sthemmin@microsoft.com> Make the KNI documentation reflect modern kernel networking. Ifconfig has been superseded by iproute2 for 15 years. Iproute2 is well maintained, supports current feature set. Ethtool is no longer supported by KNI. Tshark is a better replacement for tcpdump. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- .../sample_app_ug/kernel_nic_interface.rst | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/doc/guides/sample_app_ug/kernel_nic_interface.rst b/doc/guides/sample_app_ug/kernel_nic_interface.rst index a7e549d5213a..422bd8c98465 100644 --- a/doc/guides/sample_app_ug/kernel_nic_interface.rst +++ b/doc/guides/sample_app_ug/kernel_nic_interface.rst @@ -21,14 +21,14 @@ The FIFO queues contain pointers to data packets in the DPDK. This: * Provides a faster mechanism to interface with the kernel net stack and eliminates system calls -* Facilitates the DPDK using standard Linux* userspace net tools (tcpdump, ftp, and so on) +* Facilitates the DPDK using standard Linux* userspace net tools (tshark, rsync, and so on) * Eliminate the copy_to_user and copy_from_user operations on packets. The Kernel NIC Interface sample application is a simple example that demonstrates the use of the DPDK to create a path for packets to go through the Linux* kernel. This is done by creating one or more kernel net devices for each of the DPDK ports. -The application allows the use of standard Linux tools (ethtool, ifconfig, tcpdump) with the DPDK ports and +The application allows the use of standard Linux tools (iproute, tshark) with the DPDK ports and also the exchange of packets between the DPDK application and the Linux* kernel. The Kernel NIC Interface sample application requires that the @@ -220,13 +220,13 @@ Enable KNI interface and assign an IP address: .. code-block:: console - # ifconfig vEth0_0 192.168.0.1 + # ip addr add dev vEth0_0 192.168.0.1 Show KNI interface configuration and statistics: .. code-block:: console - # ifconfig vEth0_0 + # ip -s -d addr show vEth0_0 The user can also check and reset the packet statistics inside the ``kni`` application by sending the app the USR1 and USR2 signals: @@ -234,16 +234,16 @@ application by sending the app the USR1 and USR2 signals: .. code-block:: console # Print statistics - # kill -SIGUSR1 `pidof kni` + # pkill -USR1 kni # Zero statistics - # kill -SIGUSR2 `pidof kni` + # pkill -USR2 kni Dump network traffic: .. code-block:: console - # tcpdump -i vEth0_0 + # tshark -n -i vEth0_0 The normal Linux commands can also be used to change the MAC address and MTU size used by the physical NIC which corresponds to the KNI interface. @@ -254,13 +254,13 @@ Change the MAC address: .. code-block:: console - # ifconfig vEth0_0 hw ether 0C:01:02:03:04:08 + # ip link set dev vEth0_0 lladdr 0C:01:02:03:04:08 Change the MTU size: .. code-block:: console - # ifconfig vEth0_0 mtu 1450 + # ip link set dev vEth0_0 mtu 1450 When the ``kni`` application is closed, all the KNI interfaces are deleted from the Linux kernel. -- 2.20.1 ^ permalink raw reply [flat|nested] 63+ messages in thread
* [dpdk-dev] [PATCH v4 8/8] kni: fix style issues 2019-06-19 18:59 ` [dpdk-dev] [PATCH v4 0/8] kni: fixes and cleanups Stephen Hemminger ` (6 preceding siblings ...) 2019-06-19 18:59 ` [dpdk-dev] [PATCH v4 7/8] doc: update KNI documentation Stephen Hemminger @ 2019-06-19 18:59 ` Stephen Hemminger 7 siblings, 0 replies; 63+ messages in thread From: Stephen Hemminger @ 2019-06-19 18:59 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger, Stephen Hemminger From: Stephen Hemminger <sthemmin@microsoft.com> rte_kni does not follow standard style rules. Noticed some extra \ line continuation etc. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- lib/librte_kni/rte_kni.c | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/lib/librte_kni/rte_kni.c b/lib/librte_kni/rte_kni.c index e29d0cc7df3c..9b6acc382fc3 100644 --- a/lib/librte_kni/rte_kni.c +++ b/lib/librte_kni/rte_kni.c @@ -59,7 +59,7 @@ struct rte_kni { uint16_t group_id; /**< Group ID of KNI devices */ uint32_t slot_id; /**< KNI pool slot ID */ struct rte_mempool *pktmbuf_pool; /**< pkt mbuf mempool */ - unsigned mbuf_size; /**< mbuf size */ + unsigned int mbuf_size; /**< mbuf size */ const struct rte_memzone *m_tx_q; /**< TX queue memzone */ const struct rte_memzone *m_rx_q; /**< RX queue memzone */ @@ -78,7 +78,7 @@ struct rte_kni { /* For request & response */ struct rte_kni_fifo *req_q; /**< Request queue */ struct rte_kni_fifo *resp_q; /**< Response queue */ - void * sync_addr; /**< Req/Resp Mem address */ + void *sync_addr; /**< Req/Resp Mem address */ struct rte_kni_ops ops; /**< operations for request */ }; @@ -473,7 +473,7 @@ kni_config_promiscusity(uint16_t port_id, uint8_t to_on) int rte_kni_handle_request(struct rte_kni *kni) { - unsigned ret; + unsigned int ret; struct rte_kni_request *req = NULL; if (kni == NULL) @@ -498,8 +498,8 @@ rte_kni_handle_request(struct rte_kni *kni) break; case RTE_KNI_REQ_CFG_NETWORK_IF: /* Set network interface up/down */ if (kni->ops.config_network_if) - req->result = kni->ops.config_network_if(\ - kni->ops.port_id, req->if_up); + req->result = kni->ops.config_network_if(kni->ops.port_id, + req->if_up); break; case RTE_KNI_REQ_CHANGE_MAC_ADDR: /* Change MAC Address */ if (kni->ops.config_mac_address) @@ -534,7 +534,7 @@ rte_kni_handle_request(struct rte_kni *kni) } unsigned -rte_kni_tx_burst(struct rte_kni *kni, struct rte_mbuf **mbufs, unsigned num) +rte_kni_tx_burst(struct rte_kni *kni, struct rte_mbuf **mbufs, unsigned int num) { void *phy_mbufs[num]; unsigned int ret; @@ -552,9 +552,9 @@ rte_kni_tx_burst(struct rte_kni *kni, struct rte_mbuf **mbufs, unsigned num) } unsigned -rte_kni_rx_burst(struct rte_kni *kni, struct rte_mbuf **mbufs, unsigned num) +rte_kni_rx_burst(struct rte_kni *kni, struct rte_mbuf **mbufs, unsigned int num) { - unsigned ret = kni_fifo_get(kni->tx_q, (void **)mbufs, num); + unsigned int ret = kni_fifo_get(kni->tx_q, (void **)mbufs, num); /* If buffers removed, allocate mbufs and then put them into alloc_q */ if (ret) @@ -605,7 +605,7 @@ kni_allocate_mbufs(struct rte_kni *kni) return; } - allocq_free = (kni->alloc_q->read - kni->alloc_q->write - 1) \ + allocq_free = (kni->alloc_q->read - kni->alloc_q->write - 1) & (MAX_MBUF_BURST_NUM - 1); for (i = 0; i < allocq_free; i++) { pkts[i] = rte_pktmbuf_alloc(kni->pktmbuf_pool); @@ -659,35 +659,35 @@ static enum kni_ops_status kni_check_request_register(struct rte_kni_ops *ops) { /* check if KNI request ops has been registered*/ - if( NULL == ops ) + if (ops == NULL) return KNI_REQ_NO_REGISTER; - if ((ops->change_mtu == NULL) - && (ops->config_network_if == NULL) - && (ops->config_mac_address == NULL) - && (ops->config_promiscusity == NULL)) + if (ops->change_mtu == NULL + && ops->config_network_if == NULL + && ops->config_mac_address == NULL + && ops->config_promiscusity == NULL) return KNI_REQ_NO_REGISTER; return KNI_REQ_REGISTERED; } int -rte_kni_register_handlers(struct rte_kni *kni,struct rte_kni_ops *ops) +rte_kni_register_handlers(struct rte_kni *kni, struct rte_kni_ops *ops) { enum kni_ops_status req_status; - if (NULL == ops) { + if (ops == NULL) { RTE_LOG(ERR, KNI, "Invalid KNI request operation.\n"); return -1; } - if (NULL == kni) { + if (kni == NULL) { RTE_LOG(ERR, KNI, "Invalid kni info.\n"); return -1; } req_status = kni_check_request_register(&kni->ops); - if ( KNI_REQ_REGISTERED == req_status) { + if (req_status == KNI_REQ_REGISTERED) { RTE_LOG(ERR, KNI, "The KNI request operation has already registered.\n"); return -1; } @@ -699,7 +699,7 @@ rte_kni_register_handlers(struct rte_kni *kni,struct rte_kni_ops *ops) int rte_kni_unregister_handlers(struct rte_kni *kni) { - if (NULL == kni) { + if (kni == NULL) { RTE_LOG(ERR, KNI, "Invalid kni info.\n"); return -1; } -- 2.20.1 ^ permalink raw reply [flat|nested] 63+ messages in thread
* [dpdk-dev] [PATCH v5 0/9] kni: fixes and cleanups 2019-06-10 17:51 [dpdk-dev] [PATCH v2 0/7] kni: cleanups and improvements Stephen Hemminger ` (10 preceding siblings ...) 2019-06-19 18:59 ` [dpdk-dev] [PATCH v4 0/8] kni: fixes and cleanups Stephen Hemminger @ 2019-06-20 19:20 ` Stephen Hemminger 2019-06-20 19:20 ` [dpdk-dev] [PATCH v5 1/9] kni: don't need stubs for rx_mode or ioctl Stephen Hemminger ` (8 more replies) 2019-06-24 16:47 ` [dpdk-dev] [PATCH v6 0/9] kni: fixes and cleanups Stephen Hemminger 12 siblings, 9 replies; 63+ messages in thread From: Stephen Hemminger @ 2019-06-20 19:20 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger While testing KNI with netvsc, saw lots of places more code could be safely removed from KNI kernel driver. v5 - add minimal ethtool, fix checkpath author complaints v4 - add more style fixes v3 - rebase to current master, add style fix patch v2 - get rid of unnecessary padding, combine the unused field patches Stephen Hemminger (9): kni: don't need stubs for rx_mode or ioctl kni: use netdev_alloc_skb kni: don't keep stats in kni_net kni: drop unused fields kni: use proper type for kni fifo's kni: return -EFAULT if copy_from_user fails doc: update KNI documentation kni: fix style issues kni: add minimal ethtool .../sample_app_ug/kernel_nic_interface.rst | 18 ++-- kernel/linux/kni/kni_dev.h | 20 ++-- kernel/linux/kni/kni_misc.c | 18 ++-- kernel/linux/kni/kni_net.c | 100 +++++++----------- lib/librte_kni/rte_kni.c | 38 +++---- 5 files changed, 78 insertions(+), 116 deletions(-) -- 2.20.1 ^ permalink raw reply [flat|nested] 63+ messages in thread
* [dpdk-dev] [PATCH v5 1/9] kni: don't need stubs for rx_mode or ioctl 2019-06-20 19:20 ` [dpdk-dev] [PATCH v5 0/9] kni: fixes and cleanups Stephen Hemminger @ 2019-06-20 19:20 ` Stephen Hemminger 2019-06-20 19:20 ` [dpdk-dev] [PATCH v5 2/9] kni: use netdev_alloc_skb Stephen Hemminger ` (7 subsequent siblings) 8 siblings, 0 replies; 63+ messages in thread From: Stephen Hemminger @ 2019-06-20 19:20 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger The netdev subsystem already handles case where network sevice does not support ioctl. If device has no rx_mode hook it is not called. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- kernel/linux/kni/kni_net.c | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c index ad8365877cda..c86337d099ab 100644 --- a/kernel/linux/kni/kni_net.c +++ b/kernel/linux/kni/kni_net.c @@ -593,23 +593,6 @@ kni_net_tx_timeout(struct net_device *dev) netif_wake_queue(dev); } -/* - * Ioctl commands - */ -static int -kni_net_ioctl(struct net_device *dev, struct ifreq *rq, int cmd) -{ - pr_debug("kni_net_ioctl group:%d cmd:%d\n", - ((struct kni_dev *)netdev_priv(dev))->group_id, cmd); - - return -EOPNOTSUPP; -} - -static void -kni_net_set_rx_mode(struct net_device *dev) -{ -} - static int kni_net_change_mtu(struct net_device *dev, int new_mtu) { @@ -758,8 +741,6 @@ static const struct net_device_ops kni_net_netdev_ops = { .ndo_change_rx_flags = kni_net_set_promiscusity, .ndo_start_xmit = kni_net_tx, .ndo_change_mtu = kni_net_change_mtu, - .ndo_do_ioctl = kni_net_ioctl, - .ndo_set_rx_mode = kni_net_set_rx_mode, .ndo_get_stats = kni_net_stats, .ndo_tx_timeout = kni_net_tx_timeout, .ndo_set_mac_address = kni_net_set_mac, -- 2.20.1 ^ permalink raw reply [flat|nested] 63+ messages in thread
* [dpdk-dev] [PATCH v5 2/9] kni: use netdev_alloc_skb 2019-06-20 19:20 ` [dpdk-dev] [PATCH v5 0/9] kni: fixes and cleanups Stephen Hemminger 2019-06-20 19:20 ` [dpdk-dev] [PATCH v5 1/9] kni: don't need stubs for rx_mode or ioctl Stephen Hemminger @ 2019-06-20 19:20 ` Stephen Hemminger 2019-06-20 19:20 ` [dpdk-dev] [PATCH v5 3/9] kni: don't keep stats in kni_net Stephen Hemminger ` (6 subsequent siblings) 8 siblings, 0 replies; 63+ messages in thread From: Stephen Hemminger @ 2019-06-20 19:20 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger netdev_alloc_skb is optimized to any alignment or setup of skb->dev that is required. The kernel has chosen to not pad packets on x86 (for many years), because it is faster. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- kernel/linux/kni/kni_net.c | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c index c86337d099ab..cce5e7eb991f 100644 --- a/kernel/linux/kni/kni_net.c +++ b/kernel/linux/kni/kni_net.c @@ -340,16 +340,13 @@ kni_net_rx_normal(struct kni_dev *kni) data_kva = kva2data_kva(kva); kni->va[i] = pa2va(kni->pa[i], kva); - skb = dev_alloc_skb(len + 2); + skb = netdev_alloc_skb(dev, len); if (!skb) { /* Update statistics */ kni->stats.rx_dropped++; continue; } - /* Align IP on 16B boundary */ - skb_reserve(skb, 2); - if (kva->nb_segs == 1) { memcpy(skb_put(skb, len), data_kva, len); } else { @@ -368,7 +365,6 @@ kni_net_rx_normal(struct kni_dev *kni) } } - skb->dev = dev; skb->protocol = eth_type_trans(skb, dev); skb->ip_summed = CHECKSUM_UNNECESSARY; @@ -512,26 +508,20 @@ kni_net_rx_lo_fifo_skb(struct kni_dev *kni) data_kva = kva2data_kva(kva); kni->va[i] = pa2va(kni->pa[i], kva); - skb = dev_alloc_skb(len + 2); + skb = netdev_alloc_skb(dev, len); if (skb) { - /* Align IP on 16B boundary */ - skb_reserve(skb, 2); memcpy(skb_put(skb, len), data_kva, len); - skb->dev = dev; skb->ip_summed = CHECKSUM_UNNECESSARY; dev_kfree_skb(skb); } /* Simulate real usage, allocate/copy skb twice */ - skb = dev_alloc_skb(len + 2); + skb = netdev_alloc_skb(dev, len); if (skb == NULL) { kni->stats.rx_dropped++; continue; } - /* Align IP on 16B boundary */ - skb_reserve(skb, 2); - if (kva->nb_segs == 1) { memcpy(skb_put(skb, len), data_kva, len); } else { @@ -550,7 +540,6 @@ kni_net_rx_lo_fifo_skb(struct kni_dev *kni) } } - skb->dev = dev; skb->ip_summed = CHECKSUM_UNNECESSARY; kni->stats.rx_bytes += len; -- 2.20.1 ^ permalink raw reply [flat|nested] 63+ messages in thread
* [dpdk-dev] [PATCH v5 3/9] kni: don't keep stats in kni_net 2019-06-20 19:20 ` [dpdk-dev] [PATCH v5 0/9] kni: fixes and cleanups Stephen Hemminger 2019-06-20 19:20 ` [dpdk-dev] [PATCH v5 1/9] kni: don't need stubs for rx_mode or ioctl Stephen Hemminger 2019-06-20 19:20 ` [dpdk-dev] [PATCH v5 2/9] kni: use netdev_alloc_skb Stephen Hemminger @ 2019-06-20 19:20 ` Stephen Hemminger 2019-06-20 19:20 ` [dpdk-dev] [PATCH v5 4/9] kni: drop unused fields Stephen Hemminger ` (5 subsequent siblings) 8 siblings, 0 replies; 63+ messages in thread From: Stephen Hemminger @ 2019-06-20 19:20 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger Since kernel 2.6.28 the network subsystem has provided dev->stats for devices to use statistics handling and is the default if no ndo_get_stats is provided. This allow allows for 64 bit (rather than just 32 bit) statistics with KNI. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- kernel/linux/kni/kni_dev.h | 1 - kernel/linux/kni/kni_net.c | 43 +++++++++++++------------------------- 2 files changed, 15 insertions(+), 29 deletions(-) diff --git a/kernel/linux/kni/kni_dev.h b/kernel/linux/kni/kni_dev.h index d57bce647e4a..21e4b0d92368 100644 --- a/kernel/linux/kni/kni_dev.h +++ b/kernel/linux/kni/kni_dev.h @@ -39,7 +39,6 @@ struct kni_dev { /* kni list */ struct list_head list; - struct net_device_stats stats; int status; uint16_t group_id; /* Group ID of a group of KNI devices */ uint32_t core_id; /* Core ID to bind */ diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c index cce5e7eb991f..06310fec57bb 100644 --- a/kernel/linux/kni/kni_net.c +++ b/kernel/linux/kni/kni_net.c @@ -291,15 +291,15 @@ kni_net_tx(struct sk_buff *skb, struct net_device *dev) /* Free skb and update statistics */ dev_kfree_skb(skb); - kni->stats.tx_bytes += len; - kni->stats.tx_packets++; + dev->stats.tx_bytes += len; + dev->stats.tx_packets++; return NETDEV_TX_OK; drop: /* Free skb and update statistics */ dev_kfree_skb(skb); - kni->stats.tx_dropped++; + dev->stats.tx_dropped++; return NETDEV_TX_OK; } @@ -343,7 +343,7 @@ kni_net_rx_normal(struct kni_dev *kni) skb = netdev_alloc_skb(dev, len); if (!skb) { /* Update statistics */ - kni->stats.rx_dropped++; + dev->stats.rx_dropped++; continue; } @@ -372,8 +372,8 @@ kni_net_rx_normal(struct kni_dev *kni) netif_rx_ni(skb); /* Update statistics */ - kni->stats.rx_bytes += len; - kni->stats.rx_packets++; + dev->stats.rx_bytes += len; + dev->stats.rx_packets++; } /* Burst enqueue mbufs into free_q */ @@ -396,6 +396,7 @@ kni_net_rx_lo_fifo(struct kni_dev *kni) void *data_kva; struct rte_kni_mbuf *alloc_kva; void *alloc_data_kva; + struct net_device *dev = kni->net_dev; /* Get the number of entries in rx_q */ num_rq = kni_fifo_count(kni->rx_q); @@ -443,8 +444,8 @@ kni_net_rx_lo_fifo(struct kni_dev *kni) alloc_kva->pkt_len = len; alloc_kva->data_len = len; - kni->stats.tx_bytes += len; - kni->stats.rx_bytes += len; + dev->stats.tx_bytes += len; + dev->stats.rx_bytes += len; } /* Burst enqueue mbufs into tx_q */ @@ -464,8 +465,8 @@ kni_net_rx_lo_fifo(struct kni_dev *kni) * Update statistic, and enqueue/dequeue failure is impossible, * as all queues are checked at first. */ - kni->stats.tx_packets += num; - kni->stats.rx_packets += num; + dev->stats.tx_packets += num; + dev->stats.rx_packets += num; } /* @@ -518,7 +519,7 @@ kni_net_rx_lo_fifo_skb(struct kni_dev *kni) /* Simulate real usage, allocate/copy skb twice */ skb = netdev_alloc_skb(dev, len); if (skb == NULL) { - kni->stats.rx_dropped++; + dev->stats.rx_dropped++; continue; } @@ -542,8 +543,8 @@ kni_net_rx_lo_fifo_skb(struct kni_dev *kni) skb->ip_summed = CHECKSUM_UNNECESSARY; - kni->stats.rx_bytes += len; - kni->stats.rx_packets++; + dev->stats.rx_bytes += len; + dev->stats.rx_packets++; /* call tx interface */ kni_net_tx(skb, dev); @@ -573,12 +574,10 @@ kni_net_rx(struct kni_dev *kni) static void kni_net_tx_timeout(struct net_device *dev) { - struct kni_dev *kni = netdev_priv(dev); - pr_debug("Transmit timeout at %ld, latency %ld\n", jiffies, jiffies - dev_trans_start(dev)); - kni->stats.tx_errors++; + dev->stats.tx_errors++; netif_wake_queue(dev); } @@ -627,17 +626,6 @@ kni_net_poll_resp(struct kni_dev *kni) wake_up_interruptible(&kni->wq); } -/* - * Return statistics to the caller - */ -static struct net_device_stats * -kni_net_stats(struct net_device *dev) -{ - struct kni_dev *kni = netdev_priv(dev); - - return &kni->stats; -} - /* * Fill the eth header */ @@ -730,7 +718,6 @@ static const struct net_device_ops kni_net_netdev_ops = { .ndo_change_rx_flags = kni_net_set_promiscusity, .ndo_start_xmit = kni_net_tx, .ndo_change_mtu = kni_net_change_mtu, - .ndo_get_stats = kni_net_stats, .ndo_tx_timeout = kni_net_tx_timeout, .ndo_set_mac_address = kni_net_set_mac, #ifdef HAVE_CHANGE_CARRIER_CB -- 2.20.1 ^ permalink raw reply [flat|nested] 63+ messages in thread
* [dpdk-dev] [PATCH v5 4/9] kni: drop unused fields 2019-06-20 19:20 ` [dpdk-dev] [PATCH v5 0/9] kni: fixes and cleanups Stephen Hemminger ` (2 preceding siblings ...) 2019-06-20 19:20 ` [dpdk-dev] [PATCH v5 3/9] kni: don't keep stats in kni_net Stephen Hemminger @ 2019-06-20 19:20 ` Stephen Hemminger 2019-06-20 19:20 ` [dpdk-dev] [PATCH v5 5/9] kni: use proper type for kni fifo's Stephen Hemminger ` (4 subsequent siblings) 8 siblings, 0 replies; 63+ messages in thread From: Stephen Hemminger @ 2019-06-20 19:20 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger Several fields were either totally unused or set and never used. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- kernel/linux/kni/kni_dev.h | 5 ----- kernel/linux/kni/kni_misc.c | 1 - 2 files changed, 6 deletions(-) diff --git a/kernel/linux/kni/kni_dev.h b/kernel/linux/kni/kni_dev.h index 21e4b0d92368..f3e6c3ca4efa 100644 --- a/kernel/linux/kni/kni_dev.h +++ b/kernel/linux/kni/kni_dev.h @@ -39,8 +39,6 @@ struct kni_dev { /* kni list */ struct list_head list; - int status; - uint16_t group_id; /* Group ID of a group of KNI devices */ uint32_t core_id; /* Core ID to bind */ char name[RTE_KNI_NAMESIZE]; /* Network device name */ struct task_struct *pthread; @@ -79,9 +77,6 @@ struct kni_dev { /* mbuf size */ uint32_t mbuf_size; - /* synchro for request processing */ - unsigned long synchro; - /* buffers */ void *pa[MBUF_BURST_SZ]; void *va[MBUF_BURST_SZ]; diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c index 1fc5eeb9c8ca..b59cf24c2184 100644 --- a/kernel/linux/kni/kni_misc.c +++ b/kernel/linux/kni/kni_misc.c @@ -346,7 +346,6 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num, kni = netdev_priv(net_dev); kni->net_dev = net_dev; - kni->group_id = dev_info.group_id; kni->core_id = dev_info.core_id; strncpy(kni->name, dev_info.name, RTE_KNI_NAMESIZE); -- 2.20.1 ^ permalink raw reply [flat|nested] 63+ messages in thread
* [dpdk-dev] [PATCH v5 5/9] kni: use proper type for kni fifo's 2019-06-20 19:20 ` [dpdk-dev] [PATCH v5 0/9] kni: fixes and cleanups Stephen Hemminger ` (3 preceding siblings ...) 2019-06-20 19:20 ` [dpdk-dev] [PATCH v5 4/9] kni: drop unused fields Stephen Hemminger @ 2019-06-20 19:20 ` Stephen Hemminger 2019-06-20 19:20 ` [dpdk-dev] [PATCH v5 6/9] kni: return -EFAULT if copy_from_user fails Stephen Hemminger ` (3 subsequent siblings) 8 siblings, 0 replies; 63+ messages in thread From: Stephen Hemminger @ 2019-06-20 19:20 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger Using void * instead of proper type is unsafe practice. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- kernel/linux/kni/kni_dev.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/kernel/linux/kni/kni_dev.h b/kernel/linux/kni/kni_dev.h index f3e6c3ca4efa..ceba5f73c1d9 100644 --- a/kernel/linux/kni/kni_dev.h +++ b/kernel/linux/kni/kni_dev.h @@ -51,22 +51,22 @@ struct kni_dev { struct net_device *net_dev; /* queue for packets to be sent out */ - void *tx_q; + struct rte_kni_fifo *tx_q; /* queue for the packets received */ - void *rx_q; + struct rte_kni_fifo *rx_q; /* queue for the allocated mbufs those can be used to save sk buffs */ - void *alloc_q; + struct rte_kni_fifo *alloc_q; /* free queue for the mbufs to be freed */ - void *free_q; + struct rte_kni_fifo *free_q; /* request queue */ - void *req_q; + struct rte_kni_fifo *req_q; /* response queue */ - void *resp_q; + struct rte_kni_fifo *resp_q; void *sync_kva; void *sync_va; -- 2.20.1 ^ permalink raw reply [flat|nested] 63+ messages in thread
* [dpdk-dev] [PATCH v5 6/9] kni: return -EFAULT if copy_from_user fails 2019-06-20 19:20 ` [dpdk-dev] [PATCH v5 0/9] kni: fixes and cleanups Stephen Hemminger ` (4 preceding siblings ...) 2019-06-20 19:20 ` [dpdk-dev] [PATCH v5 5/9] kni: use proper type for kni fifo's Stephen Hemminger @ 2019-06-20 19:20 ` Stephen Hemminger 2019-06-20 19:20 ` [dpdk-dev] [PATCH v5 7/9] doc: update KNI documentation Stephen Hemminger ` (2 subsequent siblings) 8 siblings, 0 replies; 63+ messages in thread From: Stephen Hemminger @ 2019-06-20 19:20 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger The correct thing to return if user gives a bad data is to return -EFAULT. Logging is also discouraged because it could be used as a DoS attack. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- kernel/linux/kni/kni_misc.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c index b59cf24c2184..be45f823408f 100644 --- a/kernel/linux/kni/kni_misc.c +++ b/kernel/linux/kni/kni_misc.c @@ -301,11 +301,8 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num, return -EINVAL; /* Copy kni info from user space */ - ret = copy_from_user(&dev_info, (void *)ioctl_param, sizeof(dev_info)); - if (ret) { - pr_err("copy_from_user in kni_ioctl_create"); - return -EIO; - } + if (copy_from_user(&dev_info, (void *)ioctl_param, sizeof(dev_info))) + return -EFAULT; /* Check if name is zero-ended */ if (strnlen(dev_info.name, sizeof(dev_info.name)) == sizeof(dev_info.name)) { @@ -427,15 +424,12 @@ kni_ioctl_release(struct net *net, uint32_t ioctl_num, if (_IOC_SIZE(ioctl_num) > sizeof(dev_info)) return -EINVAL; - ret = copy_from_user(&dev_info, (void *)ioctl_param, sizeof(dev_info)); - if (ret) { - pr_err("copy_from_user in kni_ioctl_release"); - return -EIO; - } + if (copy_from_user(&dev_info, (void *)ioctl_param, sizeof(dev_info))) + return -EFAULT; /* Release the network device according to its name */ if (strlen(dev_info.name) == 0) - return ret; + return -EINVAL; down_write(&knet->kni_list_lock); list_for_each_entry_safe(dev, n, &knet->kni_list_head, list) { -- 2.20.1 ^ permalink raw reply [flat|nested] 63+ messages in thread
* [dpdk-dev] [PATCH v5 7/9] doc: update KNI documentation 2019-06-20 19:20 ` [dpdk-dev] [PATCH v5 0/9] kni: fixes and cleanups Stephen Hemminger ` (5 preceding siblings ...) 2019-06-20 19:20 ` [dpdk-dev] [PATCH v5 6/9] kni: return -EFAULT if copy_from_user fails Stephen Hemminger @ 2019-06-20 19:20 ` Stephen Hemminger 2019-06-20 19:20 ` [dpdk-dev] [PATCH v5 8/9] kni: fix style issues Stephen Hemminger 2019-06-20 19:20 ` [dpdk-dev] [PATCH v5 9/9] kni: add minimal ethtool Stephen Hemminger 8 siblings, 0 replies; 63+ messages in thread From: Stephen Hemminger @ 2019-06-20 19:20 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger Make the KNI documentation reflect modern kernel networking. Ifconfig has been superseded by iproute2 for 15 years. Iproute2 is well maintained, supports current feature set. Ethtool is no longer supported by KNI. Tshark is a better replacement for tcpdump. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- .../sample_app_ug/kernel_nic_interface.rst | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/doc/guides/sample_app_ug/kernel_nic_interface.rst b/doc/guides/sample_app_ug/kernel_nic_interface.rst index a7e549d5213a..422bd8c98465 100644 --- a/doc/guides/sample_app_ug/kernel_nic_interface.rst +++ b/doc/guides/sample_app_ug/kernel_nic_interface.rst @@ -21,14 +21,14 @@ The FIFO queues contain pointers to data packets in the DPDK. This: * Provides a faster mechanism to interface with the kernel net stack and eliminates system calls -* Facilitates the DPDK using standard Linux* userspace net tools (tcpdump, ftp, and so on) +* Facilitates the DPDK using standard Linux* userspace net tools (tshark, rsync, and so on) * Eliminate the copy_to_user and copy_from_user operations on packets. The Kernel NIC Interface sample application is a simple example that demonstrates the use of the DPDK to create a path for packets to go through the Linux* kernel. This is done by creating one or more kernel net devices for each of the DPDK ports. -The application allows the use of standard Linux tools (ethtool, ifconfig, tcpdump) with the DPDK ports and +The application allows the use of standard Linux tools (iproute, tshark) with the DPDK ports and also the exchange of packets between the DPDK application and the Linux* kernel. The Kernel NIC Interface sample application requires that the @@ -220,13 +220,13 @@ Enable KNI interface and assign an IP address: .. code-block:: console - # ifconfig vEth0_0 192.168.0.1 + # ip addr add dev vEth0_0 192.168.0.1 Show KNI interface configuration and statistics: .. code-block:: console - # ifconfig vEth0_0 + # ip -s -d addr show vEth0_0 The user can also check and reset the packet statistics inside the ``kni`` application by sending the app the USR1 and USR2 signals: @@ -234,16 +234,16 @@ application by sending the app the USR1 and USR2 signals: .. code-block:: console # Print statistics - # kill -SIGUSR1 `pidof kni` + # pkill -USR1 kni # Zero statistics - # kill -SIGUSR2 `pidof kni` + # pkill -USR2 kni Dump network traffic: .. code-block:: console - # tcpdump -i vEth0_0 + # tshark -n -i vEth0_0 The normal Linux commands can also be used to change the MAC address and MTU size used by the physical NIC which corresponds to the KNI interface. @@ -254,13 +254,13 @@ Change the MAC address: .. code-block:: console - # ifconfig vEth0_0 hw ether 0C:01:02:03:04:08 + # ip link set dev vEth0_0 lladdr 0C:01:02:03:04:08 Change the MTU size: .. code-block:: console - # ifconfig vEth0_0 mtu 1450 + # ip link set dev vEth0_0 mtu 1450 When the ``kni`` application is closed, all the KNI interfaces are deleted from the Linux kernel. -- 2.20.1 ^ permalink raw reply [flat|nested] 63+ messages in thread
* [dpdk-dev] [PATCH v5 8/9] kni: fix style issues 2019-06-20 19:20 ` [dpdk-dev] [PATCH v5 0/9] kni: fixes and cleanups Stephen Hemminger ` (6 preceding siblings ...) 2019-06-20 19:20 ` [dpdk-dev] [PATCH v5 7/9] doc: update KNI documentation Stephen Hemminger @ 2019-06-20 19:20 ` Stephen Hemminger 2019-06-20 19:20 ` [dpdk-dev] [PATCH v5 9/9] kni: add minimal ethtool Stephen Hemminger 8 siblings, 0 replies; 63+ messages in thread From: Stephen Hemminger @ 2019-06-20 19:20 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger rte_kni does not follow standard style rules. Noticed some extra \ line continuation etc. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- kernel/linux/kni/kni_net.c | 7 ++++--- lib/librte_kni/rte_kni.c | 38 +++++++++++++++++++------------------- 2 files changed, 23 insertions(+), 22 deletions(-) diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c index 06310fec57bb..320d51d7fc83 100644 --- a/kernel/linux/kni/kni_net.c +++ b/kernel/linux/kni/kni_net.c @@ -401,7 +401,7 @@ kni_net_rx_lo_fifo(struct kni_dev *kni) /* Get the number of entries in rx_q */ num_rq = kni_fifo_count(kni->rx_q); - /* Get the number of free entrie in tx_q */ + /* Get the number of free entries in tx_q */ num_tq = kni_fifo_free_count(kni->tx_q); /* Get the number of entries in alloc_q */ @@ -755,6 +755,7 @@ kni_net_config_lo_mode(char *lo_str) } else if (!strcmp(lo_str, "lo_mode_fifo_skb")) { pr_debug("loopback mode=lo_mode_fifo_skb enabled"); kni_net_rx_func = kni_net_rx_lo_fifo_skb; - } else - pr_debug("Incognizant parameter, loopback disabled"); + } else { + pr_debug("Unknown loopback parameter, disabled"); + } } diff --git a/lib/librte_kni/rte_kni.c b/lib/librte_kni/rte_kni.c index e29d0cc7df3c..9b6acc382fc3 100644 --- a/lib/librte_kni/rte_kni.c +++ b/lib/librte_kni/rte_kni.c @@ -59,7 +59,7 @@ struct rte_kni { uint16_t group_id; /**< Group ID of KNI devices */ uint32_t slot_id; /**< KNI pool slot ID */ struct rte_mempool *pktmbuf_pool; /**< pkt mbuf mempool */ - unsigned mbuf_size; /**< mbuf size */ + unsigned int mbuf_size; /**< mbuf size */ const struct rte_memzone *m_tx_q; /**< TX queue memzone */ const struct rte_memzone *m_rx_q; /**< RX queue memzone */ @@ -78,7 +78,7 @@ struct rte_kni { /* For request & response */ struct rte_kni_fifo *req_q; /**< Request queue */ struct rte_kni_fifo *resp_q; /**< Response queue */ - void * sync_addr; /**< Req/Resp Mem address */ + void *sync_addr; /**< Req/Resp Mem address */ struct rte_kni_ops ops; /**< operations for request */ }; @@ -473,7 +473,7 @@ kni_config_promiscusity(uint16_t port_id, uint8_t to_on) int rte_kni_handle_request(struct rte_kni *kni) { - unsigned ret; + unsigned int ret; struct rte_kni_request *req = NULL; if (kni == NULL) @@ -498,8 +498,8 @@ rte_kni_handle_request(struct rte_kni *kni) break; case RTE_KNI_REQ_CFG_NETWORK_IF: /* Set network interface up/down */ if (kni->ops.config_network_if) - req->result = kni->ops.config_network_if(\ - kni->ops.port_id, req->if_up); + req->result = kni->ops.config_network_if(kni->ops.port_id, + req->if_up); break; case RTE_KNI_REQ_CHANGE_MAC_ADDR: /* Change MAC Address */ if (kni->ops.config_mac_address) @@ -534,7 +534,7 @@ rte_kni_handle_request(struct rte_kni *kni) } unsigned -rte_kni_tx_burst(struct rte_kni *kni, struct rte_mbuf **mbufs, unsigned num) +rte_kni_tx_burst(struct rte_kni *kni, struct rte_mbuf **mbufs, unsigned int num) { void *phy_mbufs[num]; unsigned int ret; @@ -552,9 +552,9 @@ rte_kni_tx_burst(struct rte_kni *kni, struct rte_mbuf **mbufs, unsigned num) } unsigned -rte_kni_rx_burst(struct rte_kni *kni, struct rte_mbuf **mbufs, unsigned num) +rte_kni_rx_burst(struct rte_kni *kni, struct rte_mbuf **mbufs, unsigned int num) { - unsigned ret = kni_fifo_get(kni->tx_q, (void **)mbufs, num); + unsigned int ret = kni_fifo_get(kni->tx_q, (void **)mbufs, num); /* If buffers removed, allocate mbufs and then put them into alloc_q */ if (ret) @@ -605,7 +605,7 @@ kni_allocate_mbufs(struct rte_kni *kni) return; } - allocq_free = (kni->alloc_q->read - kni->alloc_q->write - 1) \ + allocq_free = (kni->alloc_q->read - kni->alloc_q->write - 1) & (MAX_MBUF_BURST_NUM - 1); for (i = 0; i < allocq_free; i++) { pkts[i] = rte_pktmbuf_alloc(kni->pktmbuf_pool); @@ -659,35 +659,35 @@ static enum kni_ops_status kni_check_request_register(struct rte_kni_ops *ops) { /* check if KNI request ops has been registered*/ - if( NULL == ops ) + if (ops == NULL) return KNI_REQ_NO_REGISTER; - if ((ops->change_mtu == NULL) - && (ops->config_network_if == NULL) - && (ops->config_mac_address == NULL) - && (ops->config_promiscusity == NULL)) + if (ops->change_mtu == NULL + && ops->config_network_if == NULL + && ops->config_mac_address == NULL + && ops->config_promiscusity == NULL) return KNI_REQ_NO_REGISTER; return KNI_REQ_REGISTERED; } int -rte_kni_register_handlers(struct rte_kni *kni,struct rte_kni_ops *ops) +rte_kni_register_handlers(struct rte_kni *kni, struct rte_kni_ops *ops) { enum kni_ops_status req_status; - if (NULL == ops) { + if (ops == NULL) { RTE_LOG(ERR, KNI, "Invalid KNI request operation.\n"); return -1; } - if (NULL == kni) { + if (kni == NULL) { RTE_LOG(ERR, KNI, "Invalid kni info.\n"); return -1; } req_status = kni_check_request_register(&kni->ops); - if ( KNI_REQ_REGISTERED == req_status) { + if (req_status == KNI_REQ_REGISTERED) { RTE_LOG(ERR, KNI, "The KNI request operation has already registered.\n"); return -1; } @@ -699,7 +699,7 @@ rte_kni_register_handlers(struct rte_kni *kni,struct rte_kni_ops *ops) int rte_kni_unregister_handlers(struct rte_kni *kni) { - if (NULL == kni) { + if (kni == NULL) { RTE_LOG(ERR, KNI, "Invalid kni info.\n"); return -1; } -- 2.20.1 ^ permalink raw reply [flat|nested] 63+ messages in thread
* [dpdk-dev] [PATCH v5 9/9] kni: add minimal ethtool 2019-06-20 19:20 ` [dpdk-dev] [PATCH v5 0/9] kni: fixes and cleanups Stephen Hemminger ` (7 preceding siblings ...) 2019-06-20 19:20 ` [dpdk-dev] [PATCH v5 8/9] kni: fix style issues Stephen Hemminger @ 2019-06-20 19:20 ` Stephen Hemminger 2019-06-21 8:26 ` Igor Ryzhov 8 siblings, 1 reply; 63+ messages in thread From: Stephen Hemminger @ 2019-06-20 19:20 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger Some applications use ethtool so add the minimum ethtool ops. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- kernel/linux/kni/kni_dev.h | 2 ++ kernel/linux/kni/kni_misc.c | 1 + kernel/linux/kni/kni_net.c | 14 ++++++++++++++ 3 files changed, 17 insertions(+) diff --git a/kernel/linux/kni/kni_dev.h b/kernel/linux/kni/kni_dev.h index ceba5f73c1d9..c1ca6789ce12 100644 --- a/kernel/linux/kni/kni_dev.h +++ b/kernel/linux/kni/kni_dev.h @@ -11,6 +11,8 @@ #endif #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt +#define KNI_VERSION "1.0" + #include "compat.h" #include <linux/if.h> diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c index be45f823408f..2b75502a8b0e 100644 --- a/kernel/linux/kni/kni_misc.c +++ b/kernel/linux/kni/kni_misc.c @@ -21,6 +21,7 @@ #include "compat.h" #include "kni_dev.h" +MODULE_VERSION(KNI_VERSION); MODULE_LICENSE("Dual BSD/GPL"); MODULE_AUTHOR("Intel Corporation"); MODULE_DESCRIPTION("Kernel Module for managing kni devices"); diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c index 320d51d7fc83..319ee2dcb19a 100644 --- a/kernel/linux/kni/kni_net.c +++ b/kernel/linux/kni/kni_net.c @@ -13,6 +13,7 @@ #include <linux/version.h> #include <linux/netdevice.h> #include <linux/etherdevice.h> /* eth_type_trans */ +#include <linux/ethtool.h> #include <linux/skbuff.h> #include <linux/kthread.h> #include <linux/delay.h> @@ -725,6 +726,18 @@ static const struct net_device_ops kni_net_netdev_ops = { #endif }; +static void kni_get_drvinfo(struct net_device *dev, + struct ethtool_drvinfo *info) +{ + strlcpy(info->version, KNI_VERSION, sizeof(info->version)); + strlcpy(info->driver, "kni", sizeof(info->driver)); +} + +static const struct ethtool_ops kni_net_ethtool_ops = { + .get_drvinfo = kni_get_drvinfo, + .get_link = ethtool_op_get_link, +}; + void kni_net_init(struct net_device *dev) { @@ -736,6 +749,7 @@ kni_net_init(struct net_device *dev) ether_setup(dev); /* assign some of the fields */ dev->netdev_ops = &kni_net_netdev_ops; dev->header_ops = &kni_net_header_ops; + dev->ethtool_ops = &kni_net_ethtool_ops; dev->watchdog_timeo = WD_TIMEOUT; } -- 2.20.1 ^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [dpdk-dev] [PATCH v5 9/9] kni: add minimal ethtool 2019-06-20 19:20 ` [dpdk-dev] [PATCH v5 9/9] kni: add minimal ethtool Stephen Hemminger @ 2019-06-21 8:26 ` Igor Ryzhov 2019-06-21 15:10 ` Stephen Hemminger 0 siblings, 1 reply; 63+ messages in thread From: Igor Ryzhov @ 2019-06-21 8:26 UTC (permalink / raw) To: Stephen Hemminger; +Cc: dev Hi Stephen, As ethtool support is restored, I think your patch #7 should be updated to keep ethtool support in docs. Best regards, Igor On Thu, Jun 20, 2019 at 10:22 PM Stephen Hemminger <stephen@networkplumber.org> wrote: > > Some applications use ethtool so add the minimum ethtool ops. > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> > --- > kernel/linux/kni/kni_dev.h | 2 ++ > kernel/linux/kni/kni_misc.c | 1 + > kernel/linux/kni/kni_net.c | 14 ++++++++++++++ > 3 files changed, 17 insertions(+) > > diff --git a/kernel/linux/kni/kni_dev.h b/kernel/linux/kni/kni_dev.h > index ceba5f73c1d9..c1ca6789ce12 100644 > --- a/kernel/linux/kni/kni_dev.h > +++ b/kernel/linux/kni/kni_dev.h > @@ -11,6 +11,8 @@ > #endif > #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > > +#define KNI_VERSION "1.0" > + > #include "compat.h" > > #include <linux/if.h> > diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c > index be45f823408f..2b75502a8b0e 100644 > --- a/kernel/linux/kni/kni_misc.c > +++ b/kernel/linux/kni/kni_misc.c > @@ -21,6 +21,7 @@ > #include "compat.h" > #include "kni_dev.h" > > +MODULE_VERSION(KNI_VERSION); > MODULE_LICENSE("Dual BSD/GPL"); > MODULE_AUTHOR("Intel Corporation"); > MODULE_DESCRIPTION("Kernel Module for managing kni devices"); > diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c > index 320d51d7fc83..319ee2dcb19a 100644 > --- a/kernel/linux/kni/kni_net.c > +++ b/kernel/linux/kni/kni_net.c > @@ -13,6 +13,7 @@ > #include <linux/version.h> > #include <linux/netdevice.h> > #include <linux/etherdevice.h> /* eth_type_trans */ > +#include <linux/ethtool.h> > #include <linux/skbuff.h> > #include <linux/kthread.h> > #include <linux/delay.h> > @@ -725,6 +726,18 @@ static const struct net_device_ops kni_net_netdev_ops = { > #endif > }; > > +static void kni_get_drvinfo(struct net_device *dev, > + struct ethtool_drvinfo *info) > +{ > + strlcpy(info->version, KNI_VERSION, sizeof(info->version)); > + strlcpy(info->driver, "kni", sizeof(info->driver)); > +} > + > +static const struct ethtool_ops kni_net_ethtool_ops = { > + .get_drvinfo = kni_get_drvinfo, > + .get_link = ethtool_op_get_link, > +}; > + > void > kni_net_init(struct net_device *dev) > { > @@ -736,6 +749,7 @@ kni_net_init(struct net_device *dev) > ether_setup(dev); /* assign some of the fields */ > dev->netdev_ops = &kni_net_netdev_ops; > dev->header_ops = &kni_net_header_ops; > + dev->ethtool_ops = &kni_net_ethtool_ops; > dev->watchdog_timeo = WD_TIMEOUT; > } > > -- > 2.20.1 > ^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [dpdk-dev] [PATCH v5 9/9] kni: add minimal ethtool 2019-06-21 8:26 ` Igor Ryzhov @ 2019-06-21 15:10 ` Stephen Hemminger 0 siblings, 0 replies; 63+ messages in thread From: Stephen Hemminger @ 2019-06-21 15:10 UTC (permalink / raw) To: Igor Ryzhov; +Cc: dev On Fri, 21 Jun 2019 11:26:09 +0300 Igor Ryzhov <iryzhov@nfware.com> wrote: > Hi Stephen, > > As ethtool support is restored, I think your patch #7 should be > updated to keep ethtool support in docs. > > Best regards, > Igor Good idea, I will add that. This is not like the original ethtool, it is intentionally limited in scope and generic (unlike the original). ^ permalink raw reply [flat|nested] 63+ messages in thread
* [dpdk-dev] [PATCH v6 0/9] kni: fixes and cleanups 2019-06-10 17:51 [dpdk-dev] [PATCH v2 0/7] kni: cleanups and improvements Stephen Hemminger ` (11 preceding siblings ...) 2019-06-20 19:20 ` [dpdk-dev] [PATCH v5 0/9] kni: fixes and cleanups Stephen Hemminger @ 2019-06-24 16:47 ` Stephen Hemminger 2019-06-24 16:47 ` [dpdk-dev] [PATCH v6 1/9] kni: don't need stubs for rx_mode or ioctl Stephen Hemminger ` (9 more replies) 12 siblings, 10 replies; 63+ messages in thread From: Stephen Hemminger @ 2019-06-24 16:47 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger While testing KNI with netvsc, saw lots of places more code could be safely removed from KNI kernel driver. v6 - more updates to documentation v5 - add minimal ethtool, fix checkpath author complaints v4 - add more style fixes v3 - rebase to current master, add style fix patch v2 - get rid of unnecessary padding, combine the unused field patches Stephen Hemminger (9): kni: don't need stubs for rx_mode or ioctl kni: use netdev_alloc_skb kni: don't keep stats in kni_net kni: drop unused fields kni: use proper type for kni fifo's kni: return -EFAULT if copy_from_user fails kni: fix style issues kni: add minimal ethtool doc: update KNI documentation .../prog_guide/kernel_nic_interface.rst | 9 +- .../sample_app_ug/kernel_nic_interface.rst | 24 +++-- kernel/linux/kni/kni_dev.h | 20 ++-- kernel/linux/kni/kni_misc.c | 18 ++-- kernel/linux/kni/kni_net.c | 100 +++++++----------- lib/librte_kni/rte_kni.c | 38 +++---- 6 files changed, 89 insertions(+), 120 deletions(-) -- 2.20.1 ^ permalink raw reply [flat|nested] 63+ messages in thread
* [dpdk-dev] [PATCH v6 1/9] kni: don't need stubs for rx_mode or ioctl 2019-06-24 16:47 ` [dpdk-dev] [PATCH v6 0/9] kni: fixes and cleanups Stephen Hemminger @ 2019-06-24 16:47 ` Stephen Hemminger 2019-06-24 16:47 ` [dpdk-dev] [PATCH v6 2/9] kni: use netdev_alloc_skb Stephen Hemminger ` (8 subsequent siblings) 9 siblings, 0 replies; 63+ messages in thread From: Stephen Hemminger @ 2019-06-24 16:47 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger The netdev subsystem already handles case where network sevice does not support ioctl. If device has no rx_mode hook it is not called. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- kernel/linux/kni/kni_net.c | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c index ad8365877cda..c86337d099ab 100644 --- a/kernel/linux/kni/kni_net.c +++ b/kernel/linux/kni/kni_net.c @@ -593,23 +593,6 @@ kni_net_tx_timeout(struct net_device *dev) netif_wake_queue(dev); } -/* - * Ioctl commands - */ -static int -kni_net_ioctl(struct net_device *dev, struct ifreq *rq, int cmd) -{ - pr_debug("kni_net_ioctl group:%d cmd:%d\n", - ((struct kni_dev *)netdev_priv(dev))->group_id, cmd); - - return -EOPNOTSUPP; -} - -static void -kni_net_set_rx_mode(struct net_device *dev) -{ -} - static int kni_net_change_mtu(struct net_device *dev, int new_mtu) { @@ -758,8 +741,6 @@ static const struct net_device_ops kni_net_netdev_ops = { .ndo_change_rx_flags = kni_net_set_promiscusity, .ndo_start_xmit = kni_net_tx, .ndo_change_mtu = kni_net_change_mtu, - .ndo_do_ioctl = kni_net_ioctl, - .ndo_set_rx_mode = kni_net_set_rx_mode, .ndo_get_stats = kni_net_stats, .ndo_tx_timeout = kni_net_tx_timeout, .ndo_set_mac_address = kni_net_set_mac, -- 2.20.1 ^ permalink raw reply [flat|nested] 63+ messages in thread
* [dpdk-dev] [PATCH v6 2/9] kni: use netdev_alloc_skb 2019-06-24 16:47 ` [dpdk-dev] [PATCH v6 0/9] kni: fixes and cleanups Stephen Hemminger 2019-06-24 16:47 ` [dpdk-dev] [PATCH v6 1/9] kni: don't need stubs for rx_mode or ioctl Stephen Hemminger @ 2019-06-24 16:47 ` Stephen Hemminger 2019-06-24 16:47 ` [dpdk-dev] [PATCH v6 3/9] kni: don't keep stats in kni_net Stephen Hemminger ` (7 subsequent siblings) 9 siblings, 0 replies; 63+ messages in thread From: Stephen Hemminger @ 2019-06-24 16:47 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger netdev_alloc_skb is optimized to any alignment or setup of skb->dev that is required. The kernel has chosen to not pad packets on x86 (for many years), because it is faster. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- kernel/linux/kni/kni_net.c | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c index c86337d099ab..cce5e7eb991f 100644 --- a/kernel/linux/kni/kni_net.c +++ b/kernel/linux/kni/kni_net.c @@ -340,16 +340,13 @@ kni_net_rx_normal(struct kni_dev *kni) data_kva = kva2data_kva(kva); kni->va[i] = pa2va(kni->pa[i], kva); - skb = dev_alloc_skb(len + 2); + skb = netdev_alloc_skb(dev, len); if (!skb) { /* Update statistics */ kni->stats.rx_dropped++; continue; } - /* Align IP on 16B boundary */ - skb_reserve(skb, 2); - if (kva->nb_segs == 1) { memcpy(skb_put(skb, len), data_kva, len); } else { @@ -368,7 +365,6 @@ kni_net_rx_normal(struct kni_dev *kni) } } - skb->dev = dev; skb->protocol = eth_type_trans(skb, dev); skb->ip_summed = CHECKSUM_UNNECESSARY; @@ -512,26 +508,20 @@ kni_net_rx_lo_fifo_skb(struct kni_dev *kni) data_kva = kva2data_kva(kva); kni->va[i] = pa2va(kni->pa[i], kva); - skb = dev_alloc_skb(len + 2); + skb = netdev_alloc_skb(dev, len); if (skb) { - /* Align IP on 16B boundary */ - skb_reserve(skb, 2); memcpy(skb_put(skb, len), data_kva, len); - skb->dev = dev; skb->ip_summed = CHECKSUM_UNNECESSARY; dev_kfree_skb(skb); } /* Simulate real usage, allocate/copy skb twice */ - skb = dev_alloc_skb(len + 2); + skb = netdev_alloc_skb(dev, len); if (skb == NULL) { kni->stats.rx_dropped++; continue; } - /* Align IP on 16B boundary */ - skb_reserve(skb, 2); - if (kva->nb_segs == 1) { memcpy(skb_put(skb, len), data_kva, len); } else { @@ -550,7 +540,6 @@ kni_net_rx_lo_fifo_skb(struct kni_dev *kni) } } - skb->dev = dev; skb->ip_summed = CHECKSUM_UNNECESSARY; kni->stats.rx_bytes += len; -- 2.20.1 ^ permalink raw reply [flat|nested] 63+ messages in thread
* [dpdk-dev] [PATCH v6 3/9] kni: don't keep stats in kni_net 2019-06-24 16:47 ` [dpdk-dev] [PATCH v6 0/9] kni: fixes and cleanups Stephen Hemminger 2019-06-24 16:47 ` [dpdk-dev] [PATCH v6 1/9] kni: don't need stubs for rx_mode or ioctl Stephen Hemminger 2019-06-24 16:47 ` [dpdk-dev] [PATCH v6 2/9] kni: use netdev_alloc_skb Stephen Hemminger @ 2019-06-24 16:47 ` Stephen Hemminger 2019-06-24 16:47 ` [dpdk-dev] [PATCH v6 4/9] kni: drop unused fields Stephen Hemminger ` (6 subsequent siblings) 9 siblings, 0 replies; 63+ messages in thread From: Stephen Hemminger @ 2019-06-24 16:47 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger Since kernel 2.6.28 the network subsystem has provided dev->stats for devices to use statistics handling and is the default if no ndo_get_stats is provided. This allow allows for 64 bit (rather than just 32 bit) statistics with KNI. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- kernel/linux/kni/kni_dev.h | 1 - kernel/linux/kni/kni_net.c | 43 +++++++++++++------------------------- 2 files changed, 15 insertions(+), 29 deletions(-) diff --git a/kernel/linux/kni/kni_dev.h b/kernel/linux/kni/kni_dev.h index d57bce647e4a..21e4b0d92368 100644 --- a/kernel/linux/kni/kni_dev.h +++ b/kernel/linux/kni/kni_dev.h @@ -39,7 +39,6 @@ struct kni_dev { /* kni list */ struct list_head list; - struct net_device_stats stats; int status; uint16_t group_id; /* Group ID of a group of KNI devices */ uint32_t core_id; /* Core ID to bind */ diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c index cce5e7eb991f..06310fec57bb 100644 --- a/kernel/linux/kni/kni_net.c +++ b/kernel/linux/kni/kni_net.c @@ -291,15 +291,15 @@ kni_net_tx(struct sk_buff *skb, struct net_device *dev) /* Free skb and update statistics */ dev_kfree_skb(skb); - kni->stats.tx_bytes += len; - kni->stats.tx_packets++; + dev->stats.tx_bytes += len; + dev->stats.tx_packets++; return NETDEV_TX_OK; drop: /* Free skb and update statistics */ dev_kfree_skb(skb); - kni->stats.tx_dropped++; + dev->stats.tx_dropped++; return NETDEV_TX_OK; } @@ -343,7 +343,7 @@ kni_net_rx_normal(struct kni_dev *kni) skb = netdev_alloc_skb(dev, len); if (!skb) { /* Update statistics */ - kni->stats.rx_dropped++; + dev->stats.rx_dropped++; continue; } @@ -372,8 +372,8 @@ kni_net_rx_normal(struct kni_dev *kni) netif_rx_ni(skb); /* Update statistics */ - kni->stats.rx_bytes += len; - kni->stats.rx_packets++; + dev->stats.rx_bytes += len; + dev->stats.rx_packets++; } /* Burst enqueue mbufs into free_q */ @@ -396,6 +396,7 @@ kni_net_rx_lo_fifo(struct kni_dev *kni) void *data_kva; struct rte_kni_mbuf *alloc_kva; void *alloc_data_kva; + struct net_device *dev = kni->net_dev; /* Get the number of entries in rx_q */ num_rq = kni_fifo_count(kni->rx_q); @@ -443,8 +444,8 @@ kni_net_rx_lo_fifo(struct kni_dev *kni) alloc_kva->pkt_len = len; alloc_kva->data_len = len; - kni->stats.tx_bytes += len; - kni->stats.rx_bytes += len; + dev->stats.tx_bytes += len; + dev->stats.rx_bytes += len; } /* Burst enqueue mbufs into tx_q */ @@ -464,8 +465,8 @@ kni_net_rx_lo_fifo(struct kni_dev *kni) * Update statistic, and enqueue/dequeue failure is impossible, * as all queues are checked at first. */ - kni->stats.tx_packets += num; - kni->stats.rx_packets += num; + dev->stats.tx_packets += num; + dev->stats.rx_packets += num; } /* @@ -518,7 +519,7 @@ kni_net_rx_lo_fifo_skb(struct kni_dev *kni) /* Simulate real usage, allocate/copy skb twice */ skb = netdev_alloc_skb(dev, len); if (skb == NULL) { - kni->stats.rx_dropped++; + dev->stats.rx_dropped++; continue; } @@ -542,8 +543,8 @@ kni_net_rx_lo_fifo_skb(struct kni_dev *kni) skb->ip_summed = CHECKSUM_UNNECESSARY; - kni->stats.rx_bytes += len; - kni->stats.rx_packets++; + dev->stats.rx_bytes += len; + dev->stats.rx_packets++; /* call tx interface */ kni_net_tx(skb, dev); @@ -573,12 +574,10 @@ kni_net_rx(struct kni_dev *kni) static void kni_net_tx_timeout(struct net_device *dev) { - struct kni_dev *kni = netdev_priv(dev); - pr_debug("Transmit timeout at %ld, latency %ld\n", jiffies, jiffies - dev_trans_start(dev)); - kni->stats.tx_errors++; + dev->stats.tx_errors++; netif_wake_queue(dev); } @@ -627,17 +626,6 @@ kni_net_poll_resp(struct kni_dev *kni) wake_up_interruptible(&kni->wq); } -/* - * Return statistics to the caller - */ -static struct net_device_stats * -kni_net_stats(struct net_device *dev) -{ - struct kni_dev *kni = netdev_priv(dev); - - return &kni->stats; -} - /* * Fill the eth header */ @@ -730,7 +718,6 @@ static const struct net_device_ops kni_net_netdev_ops = { .ndo_change_rx_flags = kni_net_set_promiscusity, .ndo_start_xmit = kni_net_tx, .ndo_change_mtu = kni_net_change_mtu, - .ndo_get_stats = kni_net_stats, .ndo_tx_timeout = kni_net_tx_timeout, .ndo_set_mac_address = kni_net_set_mac, #ifdef HAVE_CHANGE_CARRIER_CB -- 2.20.1 ^ permalink raw reply [flat|nested] 63+ messages in thread
* [dpdk-dev] [PATCH v6 4/9] kni: drop unused fields 2019-06-24 16:47 ` [dpdk-dev] [PATCH v6 0/9] kni: fixes and cleanups Stephen Hemminger ` (2 preceding siblings ...) 2019-06-24 16:47 ` [dpdk-dev] [PATCH v6 3/9] kni: don't keep stats in kni_net Stephen Hemminger @ 2019-06-24 16:47 ` Stephen Hemminger 2019-06-24 16:47 ` [dpdk-dev] [PATCH v6 5/9] kni: use proper type for kni fifo's Stephen Hemminger ` (5 subsequent siblings) 9 siblings, 0 replies; 63+ messages in thread From: Stephen Hemminger @ 2019-06-24 16:47 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger Several fields were either totally unused or set and never used. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- kernel/linux/kni/kni_dev.h | 5 ----- kernel/linux/kni/kni_misc.c | 1 - 2 files changed, 6 deletions(-) diff --git a/kernel/linux/kni/kni_dev.h b/kernel/linux/kni/kni_dev.h index 21e4b0d92368..f3e6c3ca4efa 100644 --- a/kernel/linux/kni/kni_dev.h +++ b/kernel/linux/kni/kni_dev.h @@ -39,8 +39,6 @@ struct kni_dev { /* kni list */ struct list_head list; - int status; - uint16_t group_id; /* Group ID of a group of KNI devices */ uint32_t core_id; /* Core ID to bind */ char name[RTE_KNI_NAMESIZE]; /* Network device name */ struct task_struct *pthread; @@ -79,9 +77,6 @@ struct kni_dev { /* mbuf size */ uint32_t mbuf_size; - /* synchro for request processing */ - unsigned long synchro; - /* buffers */ void *pa[MBUF_BURST_SZ]; void *va[MBUF_BURST_SZ]; diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c index 1fc5eeb9c8ca..b59cf24c2184 100644 --- a/kernel/linux/kni/kni_misc.c +++ b/kernel/linux/kni/kni_misc.c @@ -346,7 +346,6 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num, kni = netdev_priv(net_dev); kni->net_dev = net_dev; - kni->group_id = dev_info.group_id; kni->core_id = dev_info.core_id; strncpy(kni->name, dev_info.name, RTE_KNI_NAMESIZE); -- 2.20.1 ^ permalink raw reply [flat|nested] 63+ messages in thread
* [dpdk-dev] [PATCH v6 5/9] kni: use proper type for kni fifo's 2019-06-24 16:47 ` [dpdk-dev] [PATCH v6 0/9] kni: fixes and cleanups Stephen Hemminger ` (3 preceding siblings ...) 2019-06-24 16:47 ` [dpdk-dev] [PATCH v6 4/9] kni: drop unused fields Stephen Hemminger @ 2019-06-24 16:47 ` Stephen Hemminger 2019-06-24 16:47 ` [dpdk-dev] [PATCH v6 6/9] kni: return -EFAULT if copy_from_user fails Stephen Hemminger ` (4 subsequent siblings) 9 siblings, 0 replies; 63+ messages in thread From: Stephen Hemminger @ 2019-06-24 16:47 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger Using void * instead of proper type is unsafe practice. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- kernel/linux/kni/kni_dev.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/kernel/linux/kni/kni_dev.h b/kernel/linux/kni/kni_dev.h index f3e6c3ca4efa..ceba5f73c1d9 100644 --- a/kernel/linux/kni/kni_dev.h +++ b/kernel/linux/kni/kni_dev.h @@ -51,22 +51,22 @@ struct kni_dev { struct net_device *net_dev; /* queue for packets to be sent out */ - void *tx_q; + struct rte_kni_fifo *tx_q; /* queue for the packets received */ - void *rx_q; + struct rte_kni_fifo *rx_q; /* queue for the allocated mbufs those can be used to save sk buffs */ - void *alloc_q; + struct rte_kni_fifo *alloc_q; /* free queue for the mbufs to be freed */ - void *free_q; + struct rte_kni_fifo *free_q; /* request queue */ - void *req_q; + struct rte_kni_fifo *req_q; /* response queue */ - void *resp_q; + struct rte_kni_fifo *resp_q; void *sync_kva; void *sync_va; -- 2.20.1 ^ permalink raw reply [flat|nested] 63+ messages in thread
* [dpdk-dev] [PATCH v6 6/9] kni: return -EFAULT if copy_from_user fails 2019-06-24 16:47 ` [dpdk-dev] [PATCH v6 0/9] kni: fixes and cleanups Stephen Hemminger ` (4 preceding siblings ...) 2019-06-24 16:47 ` [dpdk-dev] [PATCH v6 5/9] kni: use proper type for kni fifo's Stephen Hemminger @ 2019-06-24 16:47 ` Stephen Hemminger 2019-06-24 16:47 ` [dpdk-dev] [PATCH v6 7/9] kni: fix style issues Stephen Hemminger ` (3 subsequent siblings) 9 siblings, 0 replies; 63+ messages in thread From: Stephen Hemminger @ 2019-06-24 16:47 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger The correct thing to return if user gives a bad data is to return -EFAULT. Logging is also discouraged because it could be used as a DoS attack. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- kernel/linux/kni/kni_misc.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c index b59cf24c2184..be45f823408f 100644 --- a/kernel/linux/kni/kni_misc.c +++ b/kernel/linux/kni/kni_misc.c @@ -301,11 +301,8 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num, return -EINVAL; /* Copy kni info from user space */ - ret = copy_from_user(&dev_info, (void *)ioctl_param, sizeof(dev_info)); - if (ret) { - pr_err("copy_from_user in kni_ioctl_create"); - return -EIO; - } + if (copy_from_user(&dev_info, (void *)ioctl_param, sizeof(dev_info))) + return -EFAULT; /* Check if name is zero-ended */ if (strnlen(dev_info.name, sizeof(dev_info.name)) == sizeof(dev_info.name)) { @@ -427,15 +424,12 @@ kni_ioctl_release(struct net *net, uint32_t ioctl_num, if (_IOC_SIZE(ioctl_num) > sizeof(dev_info)) return -EINVAL; - ret = copy_from_user(&dev_info, (void *)ioctl_param, sizeof(dev_info)); - if (ret) { - pr_err("copy_from_user in kni_ioctl_release"); - return -EIO; - } + if (copy_from_user(&dev_info, (void *)ioctl_param, sizeof(dev_info))) + return -EFAULT; /* Release the network device according to its name */ if (strlen(dev_info.name) == 0) - return ret; + return -EINVAL; down_write(&knet->kni_list_lock); list_for_each_entry_safe(dev, n, &knet->kni_list_head, list) { -- 2.20.1 ^ permalink raw reply [flat|nested] 63+ messages in thread
* [dpdk-dev] [PATCH v6 7/9] kni: fix style issues 2019-06-24 16:47 ` [dpdk-dev] [PATCH v6 0/9] kni: fixes and cleanups Stephen Hemminger ` (5 preceding siblings ...) 2019-06-24 16:47 ` [dpdk-dev] [PATCH v6 6/9] kni: return -EFAULT if copy_from_user fails Stephen Hemminger @ 2019-06-24 16:47 ` Stephen Hemminger 2019-06-24 16:47 ` [dpdk-dev] [PATCH v6 8/9] kni: add minimal ethtool Stephen Hemminger ` (2 subsequent siblings) 9 siblings, 0 replies; 63+ messages in thread From: Stephen Hemminger @ 2019-06-24 16:47 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger rte_kni does not follow standard style rules. Noticed some extra \ line continuation etc. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- kernel/linux/kni/kni_net.c | 7 ++++--- lib/librte_kni/rte_kni.c | 38 +++++++++++++++++++------------------- 2 files changed, 23 insertions(+), 22 deletions(-) diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c index 06310fec57bb..320d51d7fc83 100644 --- a/kernel/linux/kni/kni_net.c +++ b/kernel/linux/kni/kni_net.c @@ -401,7 +401,7 @@ kni_net_rx_lo_fifo(struct kni_dev *kni) /* Get the number of entries in rx_q */ num_rq = kni_fifo_count(kni->rx_q); - /* Get the number of free entrie in tx_q */ + /* Get the number of free entries in tx_q */ num_tq = kni_fifo_free_count(kni->tx_q); /* Get the number of entries in alloc_q */ @@ -755,6 +755,7 @@ kni_net_config_lo_mode(char *lo_str) } else if (!strcmp(lo_str, "lo_mode_fifo_skb")) { pr_debug("loopback mode=lo_mode_fifo_skb enabled"); kni_net_rx_func = kni_net_rx_lo_fifo_skb; - } else - pr_debug("Incognizant parameter, loopback disabled"); + } else { + pr_debug("Unknown loopback parameter, disabled"); + } } diff --git a/lib/librte_kni/rte_kni.c b/lib/librte_kni/rte_kni.c index e29d0cc7df3c..9b6acc382fc3 100644 --- a/lib/librte_kni/rte_kni.c +++ b/lib/librte_kni/rte_kni.c @@ -59,7 +59,7 @@ struct rte_kni { uint16_t group_id; /**< Group ID of KNI devices */ uint32_t slot_id; /**< KNI pool slot ID */ struct rte_mempool *pktmbuf_pool; /**< pkt mbuf mempool */ - unsigned mbuf_size; /**< mbuf size */ + unsigned int mbuf_size; /**< mbuf size */ const struct rte_memzone *m_tx_q; /**< TX queue memzone */ const struct rte_memzone *m_rx_q; /**< RX queue memzone */ @@ -78,7 +78,7 @@ struct rte_kni { /* For request & response */ struct rte_kni_fifo *req_q; /**< Request queue */ struct rte_kni_fifo *resp_q; /**< Response queue */ - void * sync_addr; /**< Req/Resp Mem address */ + void *sync_addr; /**< Req/Resp Mem address */ struct rte_kni_ops ops; /**< operations for request */ }; @@ -473,7 +473,7 @@ kni_config_promiscusity(uint16_t port_id, uint8_t to_on) int rte_kni_handle_request(struct rte_kni *kni) { - unsigned ret; + unsigned int ret; struct rte_kni_request *req = NULL; if (kni == NULL) @@ -498,8 +498,8 @@ rte_kni_handle_request(struct rte_kni *kni) break; case RTE_KNI_REQ_CFG_NETWORK_IF: /* Set network interface up/down */ if (kni->ops.config_network_if) - req->result = kni->ops.config_network_if(\ - kni->ops.port_id, req->if_up); + req->result = kni->ops.config_network_if(kni->ops.port_id, + req->if_up); break; case RTE_KNI_REQ_CHANGE_MAC_ADDR: /* Change MAC Address */ if (kni->ops.config_mac_address) @@ -534,7 +534,7 @@ rte_kni_handle_request(struct rte_kni *kni) } unsigned -rte_kni_tx_burst(struct rte_kni *kni, struct rte_mbuf **mbufs, unsigned num) +rte_kni_tx_burst(struct rte_kni *kni, struct rte_mbuf **mbufs, unsigned int num) { void *phy_mbufs[num]; unsigned int ret; @@ -552,9 +552,9 @@ rte_kni_tx_burst(struct rte_kni *kni, struct rte_mbuf **mbufs, unsigned num) } unsigned -rte_kni_rx_burst(struct rte_kni *kni, struct rte_mbuf **mbufs, unsigned num) +rte_kni_rx_burst(struct rte_kni *kni, struct rte_mbuf **mbufs, unsigned int num) { - unsigned ret = kni_fifo_get(kni->tx_q, (void **)mbufs, num); + unsigned int ret = kni_fifo_get(kni->tx_q, (void **)mbufs, num); /* If buffers removed, allocate mbufs and then put them into alloc_q */ if (ret) @@ -605,7 +605,7 @@ kni_allocate_mbufs(struct rte_kni *kni) return; } - allocq_free = (kni->alloc_q->read - kni->alloc_q->write - 1) \ + allocq_free = (kni->alloc_q->read - kni->alloc_q->write - 1) & (MAX_MBUF_BURST_NUM - 1); for (i = 0; i < allocq_free; i++) { pkts[i] = rte_pktmbuf_alloc(kni->pktmbuf_pool); @@ -659,35 +659,35 @@ static enum kni_ops_status kni_check_request_register(struct rte_kni_ops *ops) { /* check if KNI request ops has been registered*/ - if( NULL == ops ) + if (ops == NULL) return KNI_REQ_NO_REGISTER; - if ((ops->change_mtu == NULL) - && (ops->config_network_if == NULL) - && (ops->config_mac_address == NULL) - && (ops->config_promiscusity == NULL)) + if (ops->change_mtu == NULL + && ops->config_network_if == NULL + && ops->config_mac_address == NULL + && ops->config_promiscusity == NULL) return KNI_REQ_NO_REGISTER; return KNI_REQ_REGISTERED; } int -rte_kni_register_handlers(struct rte_kni *kni,struct rte_kni_ops *ops) +rte_kni_register_handlers(struct rte_kni *kni, struct rte_kni_ops *ops) { enum kni_ops_status req_status; - if (NULL == ops) { + if (ops == NULL) { RTE_LOG(ERR, KNI, "Invalid KNI request operation.\n"); return -1; } - if (NULL == kni) { + if (kni == NULL) { RTE_LOG(ERR, KNI, "Invalid kni info.\n"); return -1; } req_status = kni_check_request_register(&kni->ops); - if ( KNI_REQ_REGISTERED == req_status) { + if (req_status == KNI_REQ_REGISTERED) { RTE_LOG(ERR, KNI, "The KNI request operation has already registered.\n"); return -1; } @@ -699,7 +699,7 @@ rte_kni_register_handlers(struct rte_kni *kni,struct rte_kni_ops *ops) int rte_kni_unregister_handlers(struct rte_kni *kni) { - if (NULL == kni) { + if (kni == NULL) { RTE_LOG(ERR, KNI, "Invalid kni info.\n"); return -1; } -- 2.20.1 ^ permalink raw reply [flat|nested] 63+ messages in thread
* [dpdk-dev] [PATCH v6 8/9] kni: add minimal ethtool 2019-06-24 16:47 ` [dpdk-dev] [PATCH v6 0/9] kni: fixes and cleanups Stephen Hemminger ` (6 preceding siblings ...) 2019-06-24 16:47 ` [dpdk-dev] [PATCH v6 7/9] kni: fix style issues Stephen Hemminger @ 2019-06-24 16:47 ` Stephen Hemminger 2019-06-24 16:47 ` [dpdk-dev] [PATCH v6 9/9] doc: update KNI documentation Stephen Hemminger 2019-07-12 17:03 ` [dpdk-dev] [PATCH v6 0/9] kni: fixes and cleanups Ferruh Yigit 9 siblings, 0 replies; 63+ messages in thread From: Stephen Hemminger @ 2019-06-24 16:47 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger Some applications use ethtool so add the minimum ethtool ops. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- kernel/linux/kni/kni_dev.h | 2 ++ kernel/linux/kni/kni_misc.c | 1 + kernel/linux/kni/kni_net.c | 14 ++++++++++++++ 3 files changed, 17 insertions(+) diff --git a/kernel/linux/kni/kni_dev.h b/kernel/linux/kni/kni_dev.h index ceba5f73c1d9..c1ca6789ce12 100644 --- a/kernel/linux/kni/kni_dev.h +++ b/kernel/linux/kni/kni_dev.h @@ -11,6 +11,8 @@ #endif #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt +#define KNI_VERSION "1.0" + #include "compat.h" #include <linux/if.h> diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c index be45f823408f..2b75502a8b0e 100644 --- a/kernel/linux/kni/kni_misc.c +++ b/kernel/linux/kni/kni_misc.c @@ -21,6 +21,7 @@ #include "compat.h" #include "kni_dev.h" +MODULE_VERSION(KNI_VERSION); MODULE_LICENSE("Dual BSD/GPL"); MODULE_AUTHOR("Intel Corporation"); MODULE_DESCRIPTION("Kernel Module for managing kni devices"); diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c index 320d51d7fc83..319ee2dcb19a 100644 --- a/kernel/linux/kni/kni_net.c +++ b/kernel/linux/kni/kni_net.c @@ -13,6 +13,7 @@ #include <linux/version.h> #include <linux/netdevice.h> #include <linux/etherdevice.h> /* eth_type_trans */ +#include <linux/ethtool.h> #include <linux/skbuff.h> #include <linux/kthread.h> #include <linux/delay.h> @@ -725,6 +726,18 @@ static const struct net_device_ops kni_net_netdev_ops = { #endif }; +static void kni_get_drvinfo(struct net_device *dev, + struct ethtool_drvinfo *info) +{ + strlcpy(info->version, KNI_VERSION, sizeof(info->version)); + strlcpy(info->driver, "kni", sizeof(info->driver)); +} + +static const struct ethtool_ops kni_net_ethtool_ops = { + .get_drvinfo = kni_get_drvinfo, + .get_link = ethtool_op_get_link, +}; + void kni_net_init(struct net_device *dev) { @@ -736,6 +749,7 @@ kni_net_init(struct net_device *dev) ether_setup(dev); /* assign some of the fields */ dev->netdev_ops = &kni_net_netdev_ops; dev->header_ops = &kni_net_header_ops; + dev->ethtool_ops = &kni_net_ethtool_ops; dev->watchdog_timeo = WD_TIMEOUT; } -- 2.20.1 ^ permalink raw reply [flat|nested] 63+ messages in thread
* [dpdk-dev] [PATCH v6 9/9] doc: update KNI documentation 2019-06-24 16:47 ` [dpdk-dev] [PATCH v6 0/9] kni: fixes and cleanups Stephen Hemminger ` (7 preceding siblings ...) 2019-06-24 16:47 ` [dpdk-dev] [PATCH v6 8/9] kni: add minimal ethtool Stephen Hemminger @ 2019-06-24 16:47 ` Stephen Hemminger 2019-07-12 17:03 ` [dpdk-dev] [PATCH v6 0/9] kni: fixes and cleanups Ferruh Yigit 9 siblings, 0 replies; 63+ messages in thread From: Stephen Hemminger @ 2019-06-24 16:47 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger Update KNI documentation to reflect current ethtool support. Replace references to out dated tools (ifconfig) with modern iproute2. Tshark is a better replacement for tcpdump. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- .../prog_guide/kernel_nic_interface.rst | 9 +++---- .../sample_app_ug/kernel_nic_interface.rst | 24 ++++++++++++------- 2 files changed, 20 insertions(+), 13 deletions(-) diff --git a/doc/guides/prog_guide/kernel_nic_interface.rst b/doc/guides/prog_guide/kernel_nic_interface.rst index daf87f4a8edf..5afd05c0a745 100644 --- a/doc/guides/prog_guide/kernel_nic_interface.rst +++ b/doc/guides/prog_guide/kernel_nic_interface.rst @@ -290,7 +290,8 @@ It then puts the mbuf back in the cache. Ethtool ------- -Ethtool is a Linux-specific tool with corresponding support in the kernel -where each net device must register its own callbacks for the supported operations. -The current implementation uses the igb/ixgbe modified Linux drivers for ethtool support. -Ethtool is not supported in i40e and VMs (VF or EM devices). +Ethtool is a Linux-specific tool with corresponding support in the kernel. +The current version of kni provides minimal ethtool functionality +including querying version and link state. It does not support link +control, statistics, or dumping device registers. + diff --git a/doc/guides/sample_app_ug/kernel_nic_interface.rst b/doc/guides/sample_app_ug/kernel_nic_interface.rst index a7e549d5213a..aac4ebd8d4ff 100644 --- a/doc/guides/sample_app_ug/kernel_nic_interface.rst +++ b/doc/guides/sample_app_ug/kernel_nic_interface.rst @@ -21,14 +21,14 @@ The FIFO queues contain pointers to data packets in the DPDK. This: * Provides a faster mechanism to interface with the kernel net stack and eliminates system calls -* Facilitates the DPDK using standard Linux* userspace net tools (tcpdump, ftp, and so on) +* Facilitates the DPDK using standard Linux* userspace net tools (tshark, rsync, and so on) * Eliminate the copy_to_user and copy_from_user operations on packets. The Kernel NIC Interface sample application is a simple example that demonstrates the use of the DPDK to create a path for packets to go through the Linux* kernel. This is done by creating one or more kernel net devices for each of the DPDK ports. -The application allows the use of standard Linux tools (ethtool, ifconfig, tcpdump) with the DPDK ports and +The application allows the use of standard Linux tools (ethtool, iproute, tshark) with the DPDK ports and also the exchange of packets between the DPDK application and the Linux* kernel. The Kernel NIC Interface sample application requires that the @@ -220,13 +220,13 @@ Enable KNI interface and assign an IP address: .. code-block:: console - # ifconfig vEth0_0 192.168.0.1 + # ip addr add dev vEth0_0 192.168.0.1 Show KNI interface configuration and statistics: .. code-block:: console - # ifconfig vEth0_0 + # ip -s -d addr show vEth0_0 The user can also check and reset the packet statistics inside the ``kni`` application by sending the app the USR1 and USR2 signals: @@ -234,16 +234,16 @@ application by sending the app the USR1 and USR2 signals: .. code-block:: console # Print statistics - # kill -SIGUSR1 `pidof kni` + # pkill -USR1 kni # Zero statistics - # kill -SIGUSR2 `pidof kni` + # pkill -USR2 kni Dump network traffic: .. code-block:: console - # tcpdump -i vEth0_0 + # tshark -n -i vEth0_0 The normal Linux commands can also be used to change the MAC address and MTU size used by the physical NIC which corresponds to the KNI interface. @@ -254,13 +254,19 @@ Change the MAC address: .. code-block:: console - # ifconfig vEth0_0 hw ether 0C:01:02:03:04:08 + # ip link set dev vEth0_0 lladdr 0C:01:02:03:04:08 Change the MTU size: .. code-block:: console - # ifconfig vEth0_0 mtu 1450 + # ip link set dev vEth0_0 mtu 1450 + +Limited ethtool support: + +.. code-block:: console + + # ethtool -i vEth0_0 When the ``kni`` application is closed, all the KNI interfaces are deleted from the Linux kernel. -- 2.20.1 ^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [dpdk-dev] [PATCH v6 0/9] kni: fixes and cleanups 2019-06-24 16:47 ` [dpdk-dev] [PATCH v6 0/9] kni: fixes and cleanups Stephen Hemminger ` (8 preceding siblings ...) 2019-06-24 16:47 ` [dpdk-dev] [PATCH v6 9/9] doc: update KNI documentation Stephen Hemminger @ 2019-07-12 17:03 ` Ferruh Yigit 2019-07-15 17:18 ` Thomas Monjalon 9 siblings, 1 reply; 63+ messages in thread From: Ferruh Yigit @ 2019-07-12 17:03 UTC (permalink / raw) To: Stephen Hemminger, Thomas Monjalon; +Cc: dev On 6/24/2019 5:47 PM, Stephen Hemminger wrote: > While testing KNI with netvsc, saw lots of places more code > could be safely removed from KNI kernel driver. > > v6 - more updates to documentation > v5 - add minimal ethtool, fix checkpath author complaints > v4 - add more style fixes > v3 - rebase to current master, add style fix patch > v2 - get rid of unnecessary padding, combine the unused field patches > > Stephen Hemminger (9): > kni: don't need stubs for rx_mode or ioctl > kni: use netdev_alloc_skb > kni: don't keep stats in kni_net > kni: drop unused fields > kni: use proper type for kni fifo's > kni: return -EFAULT if copy_from_user fails > kni: fix style issues > kni: add minimal ethtool > doc: update KNI documentation For series, Acked-by: Ferruh Yigit <ferruh.yigit@intel.com> Thanks for the cleanup. I am OK to get the set for rc2, but if we will do this please lets merge this early so that we can still have time to test and fix any possible issues before rc2. ^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [dpdk-dev] [PATCH v6 0/9] kni: fixes and cleanups 2019-07-12 17:03 ` [dpdk-dev] [PATCH v6 0/9] kni: fixes and cleanups Ferruh Yigit @ 2019-07-15 17:18 ` Thomas Monjalon 0 siblings, 0 replies; 63+ messages in thread From: Thomas Monjalon @ 2019-07-15 17:18 UTC (permalink / raw) To: Stephen Hemminger; +Cc: dev, Ferruh Yigit 12/07/2019 19:03, Ferruh Yigit: > On 6/24/2019 5:47 PM, Stephen Hemminger wrote: > > While testing KNI with netvsc, saw lots of places more code > > could be safely removed from KNI kernel driver. > > > > v6 - more updates to documentation > > v5 - add minimal ethtool, fix checkpath author complaints > > v4 - add more style fixes > > v3 - rebase to current master, add style fix patch > > v2 - get rid of unnecessary padding, combine the unused field patches > > > > Stephen Hemminger (9): > > kni: don't need stubs for rx_mode or ioctl > > kni: use netdev_alloc_skb > > kni: don't keep stats in kni_net > > kni: drop unused fields > > kni: use proper type for kni fifo's > > kni: return -EFAULT if copy_from_user fails > > kni: fix style issues > > kni: add minimal ethtool > > doc: update KNI documentation > > For series, > Acked-by: Ferruh Yigit <ferruh.yigit@intel.com> > > Thanks for the cleanup. > > I am OK to get the set for rc2, but if we will do this please lets merge this > early so that we can still have time to test and fix any possible issues before rc2. Applied, thanks ^ permalink raw reply [flat|nested] 63+ messages in thread
end of thread, other threads:[~2019-07-15 17:18 UTC | newest] Thread overview: 63+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-06-10 17:51 [dpdk-dev] [PATCH v2 0/7] kni: cleanups and improvements Stephen Hemminger 2019-06-10 17:51 ` [dpdk-dev] [PATCH v2 1/7] kni: don't need stubs for rx_mode or ioctl Stephen Hemminger 2019-06-10 17:51 ` [dpdk-dev] [PATCH v2 2/7] kni: use netdev_alloc_skb Stephen Hemminger 2019-06-10 17:51 ` [dpdk-dev] [PATCH v2 3/7] kni: don't keep stats in kni_net Stephen Hemminger 2019-06-10 17:51 ` [dpdk-dev] [PATCH v2 4/7] kni: drop unused fields Stephen Hemminger 2019-06-10 17:51 ` [dpdk-dev] [PATCH v2 5/7] kni: use proper type for kni fifo's Stephen Hemminger 2019-06-10 17:51 ` [dpdk-dev] [PATCH v2 6/7] kni: return -EFAULT if copy_from_user fails Stephen Hemminger 2019-06-10 17:51 ` [dpdk-dev] [PATCH v2 7/7] doc: update KNI documentation Stephen Hemminger 2019-06-11 20:54 ` [dpdk-dev] [PATCH v2 0/7] kni: cleanups and improvements Stephen Hemminger 2019-06-11 21:18 ` Lance Richardson 2019-06-11 21:30 ` Stephen Hemminger 2019-06-11 21:49 ` Lance Richardson 2019-06-18 16:16 ` [dpdk-dev] [PATCH v3 0/8] kni: fixes and cleanups Stephen Hemminger 2019-06-18 16:16 ` [dpdk-dev] [PATCH v3 1/8] kni: don't need stubs for rx_mode or ioctl Stephen Hemminger 2019-06-18 16:16 ` [dpdk-dev] [PATCH v3 2/8] kni: use netdev_alloc_skb Stephen Hemminger 2019-06-18 16:16 ` [dpdk-dev] [PATCH v3 3/8] kni: don't keep stats in kni_net Stephen Hemminger 2019-06-18 16:16 ` [dpdk-dev] [PATCH v3 4/8] kni: drop unused fields Stephen Hemminger 2019-06-18 16:16 ` [dpdk-dev] [PATCH v3 5/8] kni: use proper type for kni fifo's Stephen Hemminger 2019-06-18 16:16 ` [dpdk-dev] [PATCH v3 6/8] kni: return -EFAULT if copy_from_user fails Stephen Hemminger 2019-06-18 16:16 ` [dpdk-dev] [PATCH v3 7/8] doc: update KNI documentation Stephen Hemminger 2019-06-18 16:16 ` [dpdk-dev] [PATCH v3 8/8] kni: fix style issues Stephen Hemminger 2019-06-19 18:57 ` [dpdk-dev] [PATCH v3 0/8] kni: cleanups and fixes Stephen Hemminger 2019-06-19 18:57 ` [dpdk-dev] [PATCH v3 1/8] kni: don't need stubs for rx_mode or ioctl Stephen Hemminger 2019-06-19 18:57 ` [dpdk-dev] [PATCH v3 2/8] kni: use netdev_alloc_skb Stephen Hemminger 2019-06-19 18:57 ` [dpdk-dev] [PATCH v3 3/8] kni: don't keep stats in kni_net Stephen Hemminger 2019-06-19 18:57 ` [dpdk-dev] [PATCH v3 4/8] kni: drop unused fields Stephen Hemminger 2019-06-19 18:57 ` [dpdk-dev] [PATCH v3 5/8] kni: use proper type for kni fifo's Stephen Hemminger 2019-06-19 18:57 ` [dpdk-dev] [PATCH v3 6/8] kni: return -EFAULT if copy_from_user fails Stephen Hemminger 2019-06-19 18:57 ` [dpdk-dev] [PATCH v3 7/8] doc: update KNI documentation Stephen Hemminger 2019-06-19 18:57 ` [dpdk-dev] [PATCH v3 8/8] kni: fix style issues Stephen Hemminger 2019-06-19 18:59 ` [dpdk-dev] [PATCH v4 0/8] kni: fixes and cleanups Stephen Hemminger 2019-06-19 18:59 ` [dpdk-dev] [PATCH v4 1/8] kni: don't need stubs for rx_mode or ioctl Stephen Hemminger 2019-06-19 18:59 ` [dpdk-dev] [PATCH v4 2/8] kni: use netdev_alloc_skb Stephen Hemminger 2019-06-19 18:59 ` [dpdk-dev] [PATCH v4 3/8] kni: don't keep stats in kni_net Stephen Hemminger 2019-06-19 18:59 ` [dpdk-dev] [PATCH v4 4/8] kni: drop unused fields Stephen Hemminger 2019-06-19 18:59 ` [dpdk-dev] [PATCH v4 5/8] kni: use proper type for kni fifo's Stephen Hemminger 2019-06-19 18:59 ` [dpdk-dev] [PATCH v4 6/8] kni: return -EFAULT if copy_from_user fails Stephen Hemminger 2019-06-19 18:59 ` [dpdk-dev] [PATCH v4 7/8] doc: update KNI documentation Stephen Hemminger 2019-06-19 18:59 ` [dpdk-dev] [PATCH v4 8/8] kni: fix style issues Stephen Hemminger 2019-06-20 19:20 ` [dpdk-dev] [PATCH v5 0/9] kni: fixes and cleanups Stephen Hemminger 2019-06-20 19:20 ` [dpdk-dev] [PATCH v5 1/9] kni: don't need stubs for rx_mode or ioctl Stephen Hemminger 2019-06-20 19:20 ` [dpdk-dev] [PATCH v5 2/9] kni: use netdev_alloc_skb Stephen Hemminger 2019-06-20 19:20 ` [dpdk-dev] [PATCH v5 3/9] kni: don't keep stats in kni_net Stephen Hemminger 2019-06-20 19:20 ` [dpdk-dev] [PATCH v5 4/9] kni: drop unused fields Stephen Hemminger 2019-06-20 19:20 ` [dpdk-dev] [PATCH v5 5/9] kni: use proper type for kni fifo's Stephen Hemminger 2019-06-20 19:20 ` [dpdk-dev] [PATCH v5 6/9] kni: return -EFAULT if copy_from_user fails Stephen Hemminger 2019-06-20 19:20 ` [dpdk-dev] [PATCH v5 7/9] doc: update KNI documentation Stephen Hemminger 2019-06-20 19:20 ` [dpdk-dev] [PATCH v5 8/9] kni: fix style issues Stephen Hemminger 2019-06-20 19:20 ` [dpdk-dev] [PATCH v5 9/9] kni: add minimal ethtool Stephen Hemminger 2019-06-21 8:26 ` Igor Ryzhov 2019-06-21 15:10 ` Stephen Hemminger 2019-06-24 16:47 ` [dpdk-dev] [PATCH v6 0/9] kni: fixes and cleanups Stephen Hemminger 2019-06-24 16:47 ` [dpdk-dev] [PATCH v6 1/9] kni: don't need stubs for rx_mode or ioctl Stephen Hemminger 2019-06-24 16:47 ` [dpdk-dev] [PATCH v6 2/9] kni: use netdev_alloc_skb Stephen Hemminger 2019-06-24 16:47 ` [dpdk-dev] [PATCH v6 3/9] kni: don't keep stats in kni_net Stephen Hemminger 2019-06-24 16:47 ` [dpdk-dev] [PATCH v6 4/9] kni: drop unused fields Stephen Hemminger 2019-06-24 16:47 ` [dpdk-dev] [PATCH v6 5/9] kni: use proper type for kni fifo's Stephen Hemminger 2019-06-24 16:47 ` [dpdk-dev] [PATCH v6 6/9] kni: return -EFAULT if copy_from_user fails Stephen Hemminger 2019-06-24 16:47 ` [dpdk-dev] [PATCH v6 7/9] kni: fix style issues Stephen Hemminger 2019-06-24 16:47 ` [dpdk-dev] [PATCH v6 8/9] kni: add minimal ethtool Stephen Hemminger 2019-06-24 16:47 ` [dpdk-dev] [PATCH v6 9/9] doc: update KNI documentation Stephen Hemminger 2019-07-12 17:03 ` [dpdk-dev] [PATCH v6 0/9] kni: fixes and cleanups Ferruh Yigit 2019-07-15 17:18 ` Thomas Monjalon
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).