* [PATCH net-next 0/9] udp_tunnel: add NIC RX port offload infrastructure
@ 2020-07-07 21:24 Jakub Kicinski
2020-07-07 21:24 ` [PATCH net-next 1/9] debugfs: make sure we can remove u32_array files cleanly Jakub Kicinski
` (8 more replies)
0 siblings, 9 replies; 17+ messages in thread
From: Jakub Kicinski @ 2020-07-07 21:24 UTC (permalink / raw)
To: davem
Cc: netdev, saeedm, michael.chan, edwin.peer, emil.s.tantilov,
alexander.h.duyck, jeffrey.t.kirsher, tariqt, mkubecek,
Jakub Kicinski
Kernel has a facility to notify drivers about the UDP tunnel ports
so that devices can recognize tunneled packets. This is important
mostly for RX - devices which don't support CHECKSUM_COMPLETE can
report checksums of inner packets, and compute RSS over inner headers.
Some drivers also match the UDP tunnel ports also for TX, although
doing so may lead to false positives and negatives.
Unfortunately the user experience when trying to take adavantage
of these facilities is suboptimal. First of all there is no way
for users to check which ports are offloaded. Many drivers resort
to printing messages to aid debugging, other use debugfs. Even worse
the availability of the RX features (NETIF_F_RX_UDP_TUNNEL_PORT)
is established purely on the basis of the driver having the ndos
installed. For most drivers, however, the ability to perform offloads
is contingent on device capabilities (driver support multiple device
and firmware versions). Unless driver resorts to hackish clearing
of features set incorrectly by the core - users are left guessing
whether their device really supports UDP tunnel port offload or not.
There is currently no way to indicate or configure whether RX
features include just the checksum offload or checksum and using
inner headers for RSS. Many drivers default to not using inner
headers for RSS because most implementations populate the source
port with entropy from the inner headers. This, however, is not
always the case, for example certain switches are only able to
use a fixed source port during encapsulation.
We have also seen many driver authors get the intricacies of UDP
tunnel port offloads wrong. Most commonly the drivers forget to
perform reference counting, or take sleeping locks in the callbacks.
This work tries to improve the situation by pulling the UDP tunnel
port table maintenance out of the drivers. It turns out that almost
all drivers maintain a fixed size table of ports (in most cases one
per tunnel type), so we can take care of all the refcounting in the
core, and let the driver specify if they need to sleep in the
callbacks or not. The new common implementation will also support
replacing ports - when a port is removed from a full table it will
try to find a previously missing port to take its place.
This patch only implements the core functionality along with a few
drivers I was hoping to test manually [1] along with a test based
on a netdevsim implementation. Following patches will convert all
the drivers. Once that's complete we can remove the ndos, and rely
directly on the new infrastrucutre.
Then after RSS (RXFH) is converted to netlink we can add the ability
to configure the use of inner RSS headers for UDP tunnels.
[1] Unfortunately I wasn't able to, turns out 2 of the devices
I had access to were older generation or had old FW, and they
did not actually support UDP tunnel port notifications (see
the second paragraph). The thrid device appears to program
the UDP ports correctly but it generates bad UDP checksums with
or without these patches. Long story short - I'd appreciate
reviews and testing here..
Jakub Kicinski (9):
debugfs: make sure we can remove u32_array files cleanly
udp_tunnel: re-number the offload tunnel types
udp_tunnel: add central NIC RX port offload infrastructure
ethtool: add tunnel info interface
netdevsim: add UDP tunnel port offload support
net: add a test for UDP tunnel info infra
ixgbe: convert to new udp_tunnel_nic infra
bnxt: convert to new udp_tunnel_nic infra
mlx4: convert to new udp_tunnel_nic infra
Documentation/filesystems/debugfs.rst | 12 +-
Documentation/networking/ethtool-netlink.rst | 33 +
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 133 +--
drivers/net/ethernet/broadcom/bnxt/bnxt.h | 6 -
drivers/net/ethernet/intel/ixgbe/ixgbe.h | 3 -
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 195 +---
.../net/ethernet/mellanox/mlx4/en_netdev.c | 107 +--
drivers/net/ethernet/mellanox/mlx4/mlx4_en.h | 2 -
drivers/net/geneve.c | 6 +-
drivers/net/netdevsim/Makefile | 2 +-
drivers/net/netdevsim/dev.c | 1 +
drivers/net/netdevsim/netdev.c | 12 +-
drivers/net/netdevsim/netdevsim.h | 19 +
drivers/net/netdevsim/udp_tunnels.c | 188 ++++
drivers/net/vxlan.c | 6 +-
fs/debugfs/file.c | 27 +-
include/linux/debugfs.h | 12 +-
include/linux/netdevice.h | 8 +
include/net/udp_tunnel.h | 129 ++-
include/uapi/linux/ethtool.h | 2 +
include/uapi/linux/ethtool_netlink.h | 55 ++
mm/cma.h | 3 +
mm/cma_debug.c | 7 +-
net/ethtool/Makefile | 3 +-
net/ethtool/common.c | 9 +
net/ethtool/common.h | 1 +
net/ethtool/netlink.c | 12 +
net/ethtool/netlink.h | 4 +
net/ethtool/strset.c | 5 +
net/ethtool/tunnels.c | 273 ++++++
net/ipv4/Makefile | 3 +-
net/ipv4/{udp_tunnel.c => udp_tunnel_core.c} | 0
net/ipv4/udp_tunnel_nic.c | 877 ++++++++++++++++++
net/ipv4/udp_tunnel_stub.c | 7 +
.../drivers/net/netdevsim/udp_tunnel_nic.sh | 785 ++++++++++++++++
35 files changed, 2556 insertions(+), 391 deletions(-)
create mode 100644 drivers/net/netdevsim/udp_tunnels.c
create mode 100644 net/ethtool/tunnels.c
rename net/ipv4/{udp_tunnel.c => udp_tunnel_core.c} (100%)
create mode 100644 net/ipv4/udp_tunnel_nic.c
create mode 100644 net/ipv4/udp_tunnel_stub.c
create mode 100644 tools/testing/selftests/drivers/net/netdevsim/udp_tunnel_nic.sh
--
2.26.2
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH net-next 1/9] debugfs: make sure we can remove u32_array files cleanly
2020-07-07 21:24 [PATCH net-next 0/9] udp_tunnel: add NIC RX port offload infrastructure Jakub Kicinski
@ 2020-07-07 21:24 ` Jakub Kicinski
2020-07-08 7:22 ` Greg Kroah-Hartman
2020-07-07 21:24 ` [PATCH net-next 2/9] udp_tunnel: re-number the offload tunnel types Jakub Kicinski
` (7 subsequent siblings)
8 siblings, 1 reply; 17+ messages in thread
From: Jakub Kicinski @ 2020-07-07 21:24 UTC (permalink / raw)
To: davem
Cc: netdev, saeedm, michael.chan, edwin.peer, emil.s.tantilov,
alexander.h.duyck, jeffrey.t.kirsher, tariqt, mkubecek,
Jakub Kicinski, Greg Kroah-Hartman, Marek Szyprowski,
Chucheng Luo
debugfs_create_u32_array() allocates a small structure to wrap
the data and size information about the array. If users ever
try to remove the file this leads to a leak since nothing ever
frees this wrapper.
That said there are no upstream users of debugfs_create_u32_array()
that'd remove a u32 array file (we only have one u32 array user in
CMA), so there is no real bug here.
Make callers pass a wrapper they allocated. This way the lifetime
management of the wrapper is on the caller, and we can avoid the
potential leak in debugfs.
CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
CC: Marek Szyprowski <m.szyprowski@samsung.com>
CC: Chucheng Luo <luochucheng@vivo.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
Documentation/filesystems/debugfs.rst | 12 ++++++++----
fs/debugfs/file.c | 27 +++++++--------------------
include/linux/debugfs.h | 12 +++++++++---
mm/cma.h | 3 +++
mm/cma_debug.c | 7 ++++---
5 files changed, 31 insertions(+), 30 deletions(-)
diff --git a/Documentation/filesystems/debugfs.rst b/Documentation/filesystems/debugfs.rst
index 1da7a4b7383d..728ab57a611a 100644
--- a/Documentation/filesystems/debugfs.rst
+++ b/Documentation/filesystems/debugfs.rst
@@ -185,13 +185,17 @@ byte offsets over a base for the register block.
If you want to dump an u32 array in debugfs, you can create file with::
+ struct debugfs_u32_array {
+ u32 *array;
+ u32 n_elements;
+ };
+
void debugfs_create_u32_array(const char *name, umode_t mode,
struct dentry *parent,
- u32 *array, u32 elements);
+ struct debugfs_u32_array *array);
-The "array" argument provides data, and the "elements" argument is
-the number of elements in the array. Note: Once array is created its
-size can not be changed.
+The "array" argument wraps a pointer to the array's data and the number
+of its elements. Note: Once array is created its size can not be changed.
There is a helper function to create device related seq_file::
diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index ae49a55bda00..d0ed71f37511 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -918,11 +918,6 @@ struct dentry *debugfs_create_blob(const char *name, umode_t mode,
}
EXPORT_SYMBOL_GPL(debugfs_create_blob);
-struct array_data {
- void *array;
- u32 elements;
-};
-
static size_t u32_format_array(char *buf, size_t bufsize,
u32 *array, int array_size)
{
@@ -943,8 +938,8 @@ static size_t u32_format_array(char *buf, size_t bufsize,
static int u32_array_open(struct inode *inode, struct file *file)
{
- struct array_data *data = inode->i_private;
- int size, elements = data->elements;
+ struct debugfs_u32_array *data = inode->i_private;
+ int size, elements = data->n_elements;
char *buf;
/*
@@ -959,7 +954,7 @@ static int u32_array_open(struct inode *inode, struct file *file)
buf[size] = 0;
file->private_data = buf;
- u32_format_array(buf, size, data->array, data->elements);
+ u32_format_array(buf, size, data->array, data->n_elements);
return nonseekable_open(inode, file);
}
@@ -996,8 +991,7 @@ static const struct file_operations u32_array_fops = {
* @parent: a pointer to the parent dentry for this file. This should be a
* directory dentry if set. If this parameter is %NULL, then the
* file will be created in the root of the debugfs filesystem.
- * @array: u32 array that provides data.
- * @elements: total number of elements in the array.
+ * @array: wrapper struct containing data pointer and size of the array.
*
* This function creates a file in debugfs with the given name that exports
* @array as data. If the @mode variable is so set it can be read from.
@@ -1005,17 +999,10 @@ static const struct file_operations u32_array_fops = {
* Once array is created its size can not be changed.
*/
void debugfs_create_u32_array(const char *name, umode_t mode,
- struct dentry *parent, u32 *array, u32 elements)
+ struct dentry *parent,
+ struct debugfs_u32_array *array)
{
- struct array_data *data = kmalloc(sizeof(*data), GFP_KERNEL);
-
- if (data == NULL)
- return;
-
- data->array = array;
- data->elements = elements;
-
- debugfs_create_file_unsafe(name, mode, parent, data, &u32_array_fops);
+ debugfs_create_file_unsafe(name, mode, parent, array, &u32_array_fops);
}
EXPORT_SYMBOL_GPL(debugfs_create_u32_array);
diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h
index 63cb3606dea7..851dd1f9a8a5 100644
--- a/include/linux/debugfs.h
+++ b/include/linux/debugfs.h
@@ -38,6 +38,11 @@ struct debugfs_regset32 {
struct device *dev; /* Optional device for Runtime PM */
};
+struct debugfs_u32_array {
+ u32 *array;
+ u32 n_elements;
+};
+
extern struct dentry *arch_debugfs_dir;
#define DEFINE_DEBUGFS_ATTRIBUTE(__fops, __get, __set, __fmt) \
@@ -136,7 +141,8 @@ void debugfs_print_regs32(struct seq_file *s, const struct debugfs_reg32 *regs,
int nregs, void __iomem *base, char *prefix);
void debugfs_create_u32_array(const char *name, umode_t mode,
- struct dentry *parent, u32 *array, u32 elements);
+ struct dentry *parent,
+ struct debugfs_u32_array *array);
struct dentry *debugfs_create_devm_seqfile(struct device *dev, const char *name,
struct dentry *parent,
@@ -316,8 +322,8 @@ static inline bool debugfs_initialized(void)
}
static inline void debugfs_create_u32_array(const char *name, umode_t mode,
- struct dentry *parent, u32 *array,
- u32 elements)
+ struct dentry *parent,
+ struct debugfs_u32_array *array)
{
}
diff --git a/mm/cma.h b/mm/cma.h
index 33c0b517733c..6698fa63279b 100644
--- a/mm/cma.h
+++ b/mm/cma.h
@@ -2,6 +2,8 @@
#ifndef __MM_CMA_H__
#define __MM_CMA_H__
+#include <linux/debugfs.h>
+
struct cma {
unsigned long base_pfn;
unsigned long count;
@@ -11,6 +13,7 @@ struct cma {
#ifdef CONFIG_CMA_DEBUGFS
struct hlist_head mem_head;
spinlock_t mem_head_lock;
+ struct debugfs_u32_array dfs_bitmap;
#endif
const char *name;
};
diff --git a/mm/cma_debug.c b/mm/cma_debug.c
index 4e6cbe2f586e..d5bf8aa34fdc 100644
--- a/mm/cma_debug.c
+++ b/mm/cma_debug.c
@@ -164,7 +164,6 @@ static void cma_debugfs_add_one(struct cma *cma, struct dentry *root_dentry)
{
struct dentry *tmp;
char name[16];
- int u32s;
scnprintf(name, sizeof(name), "cma-%s", cma->name);
@@ -180,8 +179,10 @@ static void cma_debugfs_add_one(struct cma *cma, struct dentry *root_dentry)
debugfs_create_file("used", 0444, tmp, cma, &cma_used_fops);
debugfs_create_file("maxchunk", 0444, tmp, cma, &cma_maxchunk_fops);
- u32s = DIV_ROUND_UP(cma_bitmap_maxno(cma), BITS_PER_BYTE * sizeof(u32));
- debugfs_create_u32_array("bitmap", 0444, tmp, (u32 *)cma->bitmap, u32s);
+ cma->dfs_bitmap.array = (u32 *)cma->bitmap;
+ cma->dfs_bitmap.n_elements = DIV_ROUND_UP(cma_bitmap_maxno(cma),
+ BITS_PER_BYTE * sizeof(u32));
+ debugfs_create_u32_array("bitmap", 0444, tmp, &cma->dfs_bitmap);
}
static int __init cma_debugfs_init(void)
--
2.26.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH net-next 2/9] udp_tunnel: re-number the offload tunnel types
2020-07-07 21:24 [PATCH net-next 0/9] udp_tunnel: add NIC RX port offload infrastructure Jakub Kicinski
2020-07-07 21:24 ` [PATCH net-next 1/9] debugfs: make sure we can remove u32_array files cleanly Jakub Kicinski
@ 2020-07-07 21:24 ` Jakub Kicinski
2020-07-07 21:24 ` [PATCH net-next 3/9] udp_tunnel: add central NIC RX port offload infrastructure Jakub Kicinski
` (6 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Jakub Kicinski @ 2020-07-07 21:24 UTC (permalink / raw)
To: davem
Cc: netdev, saeedm, michael.chan, edwin.peer, emil.s.tantilov,
alexander.h.duyck, jeffrey.t.kirsher, tariqt, mkubecek,
Jakub Kicinski
Make it possible to use tunnel types as flags more easily.
There doesn't appear to be any user using the type as an
array index, so this should make no difference.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
include/net/udp_tunnel.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/net/udp_tunnel.h b/include/net/udp_tunnel.h
index e7312ceb2794..0615e25f041c 100644
--- a/include/net/udp_tunnel.h
+++ b/include/net/udp_tunnel.h
@@ -106,9 +106,9 @@ void setup_udp_tunnel_sock(struct net *net, struct socket *sock,
* call this function to perform Tx offloads on outgoing traffic.
*/
enum udp_parsable_tunnel_type {
- UDP_TUNNEL_TYPE_VXLAN, /* RFC 7348 */
- UDP_TUNNEL_TYPE_GENEVE, /* draft-ietf-nvo3-geneve */
- UDP_TUNNEL_TYPE_VXLAN_GPE, /* draft-ietf-nvo3-vxlan-gpe */
+ UDP_TUNNEL_TYPE_VXLAN = BIT(0), /* RFC 7348 */
+ UDP_TUNNEL_TYPE_GENEVE = BIT(1), /* draft-ietf-nvo3-geneve */
+ UDP_TUNNEL_TYPE_VXLAN_GPE = BIT(2), /* draft-ietf-nvo3-vxlan-gpe */
};
struct udp_tunnel_info {
--
2.26.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH net-next 3/9] udp_tunnel: add central NIC RX port offload infrastructure
2020-07-07 21:24 [PATCH net-next 0/9] udp_tunnel: add NIC RX port offload infrastructure Jakub Kicinski
2020-07-07 21:24 ` [PATCH net-next 1/9] debugfs: make sure we can remove u32_array files cleanly Jakub Kicinski
2020-07-07 21:24 ` [PATCH net-next 2/9] udp_tunnel: re-number the offload tunnel types Jakub Kicinski
@ 2020-07-07 21:24 ` Jakub Kicinski
2020-07-07 21:24 ` [PATCH net-next 4/9] ethtool: add tunnel info interface Jakub Kicinski
` (5 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Jakub Kicinski @ 2020-07-07 21:24 UTC (permalink / raw)
To: davem
Cc: netdev, saeedm, michael.chan, edwin.peer, emil.s.tantilov,
alexander.h.duyck, jeffrey.t.kirsher, tariqt, mkubecek,
Jakub Kicinski
Cater to devices which:
(a) may want to sleep in the callbacks;
(b) only have IPv4 support;
(c) need all the programming to happen while the netdev is up.
Drivers attach UDP tunnel offload info struct to their netdevs,
where they declare how many UDP ports of various tunnel types
they support. Core takes care of tracking which ports to offload.
Use a fixed-size array since this matches what almost all drivers
do, and avoids a complexity and uncertainty around memory allocations
in an atomic context.
Make sure that tunnel drivers don't try to replay the ports when
new NIC netdev is registered. Automatic replays would mess up
reference counting, and will be removed completely once all drivers
are converted.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
drivers/net/geneve.c | 6 +-
drivers/net/vxlan.c | 6 +-
include/linux/netdevice.h | 8 +
include/net/udp_tunnel.h | 113 +++
net/ipv4/Makefile | 3 +-
net/ipv4/{udp_tunnel.c => udp_tunnel_core.c} | 0
net/ipv4/udp_tunnel_nic.c | 828 +++++++++++++++++++
net/ipv4/udp_tunnel_stub.c | 7 +
8 files changed, 966 insertions(+), 5 deletions(-)
rename net/ipv4/{udp_tunnel.c => udp_tunnel_core.c} (100%)
create mode 100644 net/ipv4/udp_tunnel_nic.c
create mode 100644 net/ipv4/udp_tunnel_stub.c
diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
index 4661ef865807..0089b9aa7eaa 100644
--- a/drivers/net/geneve.c
+++ b/drivers/net/geneve.c
@@ -1807,9 +1807,11 @@ static int geneve_netdevice_event(struct notifier_block *unused,
event == NETDEV_UDP_TUNNEL_DROP_INFO) {
geneve_offload_rx_ports(dev, event == NETDEV_UDP_TUNNEL_PUSH_INFO);
} else if (event == NETDEV_UNREGISTER) {
- geneve_offload_rx_ports(dev, false);
+ if (!dev->udp_tunnel_nic_info)
+ geneve_offload_rx_ports(dev, false);
} else if (event == NETDEV_REGISTER) {
- geneve_offload_rx_ports(dev, true);
+ if (!dev->udp_tunnel_nic_info)
+ geneve_offload_rx_ports(dev, true);
}
return NOTIFY_DONE;
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 89d85dcb200e..a43c97b13924 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -4477,10 +4477,12 @@ static int vxlan_netdevice_event(struct notifier_block *unused,
struct vxlan_net *vn = net_generic(dev_net(dev), vxlan_net_id);
if (event == NETDEV_UNREGISTER) {
- vxlan_offload_rx_ports(dev, false);
+ if (!dev->udp_tunnel_nic_info)
+ vxlan_offload_rx_ports(dev, false);
vxlan_handle_lowerdev_unregister(vn, dev);
} else if (event == NETDEV_REGISTER) {
- vxlan_offload_rx_ports(dev, true);
+ if (!dev->udp_tunnel_nic_info)
+ vxlan_offload_rx_ports(dev, true);
} else if (event == NETDEV_UDP_TUNNEL_PUSH_INFO ||
event == NETDEV_UDP_TUNNEL_DROP_INFO) {
vxlan_offload_rx_ports(dev, event == NETDEV_UDP_TUNNEL_PUSH_INFO);
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 39e28e11863c..ac2cd3f49aba 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -65,6 +65,8 @@ struct wpan_dev;
struct mpls_dev;
/* UDP Tunnel offloads */
struct udp_tunnel_info;
+struct udp_tunnel_nic_info;
+struct udp_tunnel_nic;
struct bpf_prog;
struct xdp_buff;
@@ -1836,6 +1838,10 @@ enum netdev_priv_flags {
*
* @macsec_ops: MACsec offloading ops
*
+ * @udp_tunnel_nic_info: static structure describing the UDP tunnel
+ * offload capabilities of the device
+ * @udp_tunnel_nic: UDP tunnel offload state
+ *
* FIXME: cleanup struct net_device such that network protocol info
* moves out.
*/
@@ -2134,6 +2140,8 @@ struct net_device {
/* MACsec management functions */
const struct macsec_ops *macsec_ops;
#endif
+ const struct udp_tunnel_nic_info *udp_tunnel_nic_info;
+ struct udp_tunnel_nic *udp_tunnel_nic;
};
#define to_net_dev(d) container_of(d, struct net_device, dev)
diff --git a/include/net/udp_tunnel.h b/include/net/udp_tunnel.h
index 0615e25f041c..2987730577e6 100644
--- a/include/net/udp_tunnel.h
+++ b/include/net/udp_tunnel.h
@@ -115,6 +115,7 @@ struct udp_tunnel_info {
unsigned short type;
sa_family_t sa_family;
__be16 port;
+ u8 hw_priv;
};
/* Notify network devices of offloadable types */
@@ -181,4 +182,116 @@ static inline void udp_tunnel_encap_enable(struct socket *sock)
udp_encap_enable();
}
+#define UDP_TUNNEL_NIC_MAX_TABLES 4
+
+enum udp_tunnel_nic_info_flags {
+ /* Device callbacks may sleep */
+ UDP_TUNNEL_NIC_INFO_MAY_SLEEP = BIT(0),
+ /* Device only supports offloads when it's open, all ports
+ * will be removed before close and re-added after open.
+ */
+ UDP_TUNNEL_NIC_INFO_OPEN_ONLY = BIT(1),
+ /* Device supports only IPv4 tunnels */
+ UDP_TUNNEL_NIC_INFO_IPV4_ONLY = BIT(2),
+};
+
+/**
+ * struct udp_tunnel_nic_info - driver UDP tunnel offload information
+ * @set_port: callback for adding a new port
+ * @unset_port: callback for removing a port
+ * @sync_table: callback for syncing the entire port table at once
+ * @flags: device flags from enum udp_tunnel_nic_info_flags
+ * @tables: UDP port tables this device has
+ * @tables.n_entries: number of entries in this table
+ * @tables.tunnel_types: types of tunnels this table accepts
+ *
+ * Drivers are expected to provide either @set_port and @unset_port callbacks
+ * or the @sync_table callback. Callbacks are invoked with rtnl lock held.
+ *
+ * Known limitations:
+ * - UDP tunnel port notifications are fundamentally best-effort -
+ * it is likely the driver will both see skbs which use a UDP tunnel port,
+ * while not being a tunneled skb, and tunnel skbs from other ports -
+ * drivers should only use these ports for non-critical RX-side offloads,
+ * e.g. the checksum offload;
+ * - none of the devices care about the socket family at present, so we don't
+ * track it. Please extend this code if you care.
+ */
+struct udp_tunnel_nic_info {
+ /* one-by-one */
+ int (*set_port)(struct net_device *dev,
+ unsigned int table, unsigned int entry,
+ struct udp_tunnel_info *ti);
+ int (*unset_port)(struct net_device *dev,
+ unsigned int table, unsigned int entry,
+ struct udp_tunnel_info *ti);
+
+ /* all at once */
+ int (*sync_table)(struct net_device *dev, unsigned int table);
+
+ unsigned int flags;
+
+ struct udp_tunnel_nic_table_info {
+ unsigned int n_entries;
+ unsigned int tunnel_types;
+ } tables[UDP_TUNNEL_NIC_MAX_TABLES];
+};
+
+/* UDP tunnel module dependencies
+ *
+ * Tunnel drivers are expected to have a hard dependency on the udp_tunnel
+ * module. NIC drivers are not, they just attach their
+ * struct udp_tunnel_nic_info to the netdev and wait for callbacks to come.
+ * Loading a tunnel driver will cause the udp_tunnel module to be loaded
+ * and only then will all the required state structures be allocated.
+ * Since we want a weak dependency from the drivers and the core to udp_tunnel
+ * we call things through the following stubs.
+ */
+struct udp_tunnel_nic_ops {
+ void (*get_port)(struct net_device *dev, unsigned int table,
+ unsigned int idx, struct udp_tunnel_info *ti);
+ void (*set_port_priv)(struct net_device *dev, unsigned int table,
+ unsigned int idx, u8 priv);
+ void (*add_port)(struct net_device *dev, struct udp_tunnel_info *ti);
+ void (*del_port)(struct net_device *dev, struct udp_tunnel_info *ti);
+ void (*reset_ntf)(struct net_device *dev);
+};
+
+extern const struct udp_tunnel_nic_ops *udp_tunnel_nic_ops;
+
+static inline void
+udp_tunnel_nic_get_port(struct net_device *dev, unsigned int table,
+ unsigned int idx, struct udp_tunnel_info *ti)
+{
+ if (udp_tunnel_nic_ops)
+ udp_tunnel_nic_ops->get_port(dev, table, idx, ti);
+}
+
+static inline void
+udp_tunnel_nic_set_port_priv(struct net_device *dev, unsigned int table,
+ unsigned int idx, u8 priv)
+{
+ if (udp_tunnel_nic_ops)
+ udp_tunnel_nic_ops->set_port_priv(dev, table, idx, priv);
+}
+
+static inline void
+udp_tunnel_nic_add_port(struct net_device *dev, struct udp_tunnel_info *ti)
+{
+ if (udp_tunnel_nic_ops)
+ udp_tunnel_nic_ops->add_port(dev, ti);
+}
+
+static inline void
+udp_tunnel_nic_del_port(struct net_device *dev, struct udp_tunnel_info *ti)
+{
+ if (udp_tunnel_nic_ops)
+ udp_tunnel_nic_ops->del_port(dev, ti);
+}
+
+static inline void udp_tunnel_nic_reset_ntf(struct net_device *dev)
+{
+ if (udp_tunnel_nic_ops)
+ udp_tunnel_nic_ops->reset_ntf(dev);
+}
#endif
diff --git a/net/ipv4/Makefile b/net/ipv4/Makefile
index 9e1a186a3671..5b77a46885b9 100644
--- a/net/ipv4/Makefile
+++ b/net/ipv4/Makefile
@@ -14,7 +14,7 @@ obj-y := route.o inetpeer.o protocol.o \
udp_offload.o arp.o icmp.o devinet.o af_inet.o igmp.o \
fib_frontend.o fib_semantics.o fib_trie.o fib_notifier.o \
inet_fragment.o ping.o ip_tunnel_core.o gre_offload.o \
- metrics.o netlink.o nexthop.o
+ metrics.o netlink.o nexthop.o udp_tunnel_stub.o
obj-$(CONFIG_BPFILTER) += bpfilter/
@@ -29,6 +29,7 @@ gre-y := gre_demux.o
obj-$(CONFIG_NET_FOU) += fou.o
obj-$(CONFIG_NET_IPGRE_DEMUX) += gre.o
obj-$(CONFIG_NET_IPGRE) += ip_gre.o
+udp_tunnel-y := udp_tunnel_core.o udp_tunnel_nic.o
obj-$(CONFIG_NET_UDP_TUNNEL) += udp_tunnel.o
obj-$(CONFIG_NET_IPVTI) += ip_vti.o
obj-$(CONFIG_SYN_COOKIES) += syncookies.o
diff --git a/net/ipv4/udp_tunnel.c b/net/ipv4/udp_tunnel_core.c
similarity index 100%
rename from net/ipv4/udp_tunnel.c
rename to net/ipv4/udp_tunnel_core.c
diff --git a/net/ipv4/udp_tunnel_nic.c b/net/ipv4/udp_tunnel_nic.c
new file mode 100644
index 000000000000..098fb0ebe998
--- /dev/null
+++ b/net/ipv4/udp_tunnel_nic.c
@@ -0,0 +1,828 @@
+// SPDX-License-Identifier: GPL-2.0-only
+// Copyright (c) 2020 Facebook Inc.
+
+#include <linux/netdevice.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+#include <linux/workqueue.h>
+#include <net/udp_tunnel.h>
+
+enum udp_tunnel_nic_table_entry_flags {
+ UDP_TUNNEL_NIC_ENTRY_ADD = BIT(0),
+ UDP_TUNNEL_NIC_ENTRY_DEL = BIT(1),
+ UDP_TUNNEL_NIC_ENTRY_OP_FAIL = BIT(2),
+ UDP_TUNNEL_NIC_ENTRY_FROZEN = BIT(3),
+};
+
+struct udp_tunnel_nic_table_entry {
+ __be16 port;
+ u8 type;
+ u8 use_cnt;
+ u8 flags;
+ u8 hw_priv;
+};
+
+/**
+ * struct udp_tunnel_nic - UDP tunnel port offload state
+ * @work: async work for talking to hardware from process context
+ * @dev: netdev pointer
+ * @need_sync: at least one port start changed
+ * @need_replay: space was freed, we need a replay of all ports
+ * @work_pending: @work is currently scheduled
+ * @n_tables: number of tables under @entries
+ * @missed: bitmap of tables which overflown
+ * @entries: table of tables of ports currently offloaded
+ */
+struct udp_tunnel_nic {
+ struct work_struct work;
+
+ struct net_device *dev;
+
+ u8 need_sync:1;
+ u8 need_replay:1;
+ u8 work_pending:1;
+
+ unsigned int n_tables;
+ unsigned long missed;
+ struct udp_tunnel_nic_table_entry **entries;
+};
+
+/* We ensure all work structs are done using driver state, but not the code.
+ * We need a workqueue we can flush before module gets removed.
+ */
+static struct workqueue_struct *udp_tunnel_nic_workqueue;
+
+static const char *udp_tunnel_nic_tunnel_type_name(unsigned int type)
+{
+ switch (type) {
+ case UDP_TUNNEL_TYPE_VXLAN:
+ return "vxlan";
+ case UDP_TUNNEL_TYPE_GENEVE:
+ return "geneve";
+ case UDP_TUNNEL_TYPE_VXLAN_GPE:
+ return "vxlan-gpe";
+ default:
+ return "unknown";
+ }
+}
+
+static bool
+udp_tunnel_nic_entry_is_free(struct udp_tunnel_nic_table_entry *entry)
+{
+ return entry->use_cnt == 0 && !entry->flags;
+}
+
+static bool
+udp_tunnel_nic_entry_is_frozen(struct udp_tunnel_nic_table_entry *entry)
+{
+ return entry->flags & UDP_TUNNEL_NIC_ENTRY_FROZEN;
+}
+
+static void
+udp_tunnel_nic_entry_freeze_used(struct udp_tunnel_nic_table_entry *entry)
+{
+ if (!udp_tunnel_nic_entry_is_free(entry))
+ entry->flags |= UDP_TUNNEL_NIC_ENTRY_FROZEN;
+}
+
+static void
+udp_tunnel_nic_entry_unfreeze(struct udp_tunnel_nic_table_entry *entry)
+{
+ entry->flags &= ~UDP_TUNNEL_NIC_ENTRY_FROZEN;
+}
+
+static bool
+udp_tunnel_nic_entry_is_queued(struct udp_tunnel_nic_table_entry *entry)
+{
+ return entry->flags & (UDP_TUNNEL_NIC_ENTRY_ADD |
+ UDP_TUNNEL_NIC_ENTRY_DEL);
+}
+
+static void
+udp_tunnel_nic_entry_queue(struct udp_tunnel_nic *utn,
+ struct udp_tunnel_nic_table_entry *entry,
+ unsigned int flag)
+{
+ entry->flags |= flag;
+ utn->need_sync = 1;
+}
+
+static void
+udp_tunnel_nic_ti_from_entry(struct udp_tunnel_nic_table_entry *entry,
+ struct udp_tunnel_info *ti)
+{
+ memset(ti, 0, sizeof(*ti));
+ ti->port = entry->port;
+ ti->type = entry->type;
+ ti->hw_priv = entry->hw_priv;
+}
+
+static bool
+udp_tunnel_nic_is_empty(struct net_device *dev, struct udp_tunnel_nic *utn)
+{
+ const struct udp_tunnel_nic_info *info = dev->udp_tunnel_nic_info;
+ unsigned int i, j;
+
+ for (i = 0; i < utn->n_tables; i++)
+ for (j = 0; j < info->tables[i].n_entries; j++)
+ if (!udp_tunnel_nic_entry_is_free(&utn->entries[i][j]))
+ return false;
+ return true;
+}
+
+static bool
+udp_tunnel_nic_should_replay(struct net_device *dev, struct udp_tunnel_nic *utn)
+{
+ const struct udp_tunnel_nic_table_info *table;
+ unsigned int i, j;
+
+ if (!utn->missed)
+ return false;
+
+ for (i = 0; i < utn->n_tables; i++) {
+ table = &dev->udp_tunnel_nic_info->tables[i];
+ if (!test_bit(i, &utn->missed))
+ continue;
+
+ for (j = 0; j < table->n_entries; j++)
+ if (udp_tunnel_nic_entry_is_free(&utn->entries[i][j]))
+ return true;
+ }
+
+ return false;
+}
+
+static void
+__udp_tunnel_nic_get_port(struct net_device *dev, unsigned int table,
+ unsigned int idx, struct udp_tunnel_info *ti)
+{
+ struct udp_tunnel_nic_table_entry *entry;
+ struct udp_tunnel_nic *utn;
+
+ utn = dev->udp_tunnel_nic;
+ entry = &utn->entries[table][idx];
+
+ /* This helper is used from .sync_table, we indicate empty entries
+ * by zero'ed @ti. Drivers which need to know the details of a port
+ * when it gets deleted should use the .set_port / .unset_port
+ * callbacks.
+ */
+ if (entry->use_cnt)
+ udp_tunnel_nic_ti_from_entry(entry, ti);
+ else
+ memset(ti, 0, sizeof(*ti));
+}
+
+static void
+__udp_tunnel_nic_set_port_priv(struct net_device *dev, unsigned int table,
+ unsigned int idx, u8 priv)
+{
+ dev->udp_tunnel_nic->entries[table][idx].hw_priv = priv;
+}
+
+static void
+udp_tunnel_nic_entry_update_done(struct udp_tunnel_nic_table_entry *entry,
+ int err)
+{
+ bool dodgy = entry->flags & UDP_TUNNEL_NIC_ENTRY_OP_FAIL;
+
+ WARN_ON_ONCE(entry->flags & UDP_TUNNEL_NIC_ENTRY_ADD &&
+ entry->flags & UDP_TUNNEL_NIC_ENTRY_DEL);
+
+ if (entry->flags & UDP_TUNNEL_NIC_ENTRY_ADD &&
+ (!err || (err == -EEXIST && dodgy)))
+ entry->flags &= ~UDP_TUNNEL_NIC_ENTRY_ADD;
+
+ if (entry->flags & UDP_TUNNEL_NIC_ENTRY_DEL &&
+ (!err || (err == -ENOENT && dodgy)))
+ entry->flags &= ~UDP_TUNNEL_NIC_ENTRY_DEL;
+
+ if (!err)
+ entry->flags &= ~UDP_TUNNEL_NIC_ENTRY_OP_FAIL;
+ else
+ entry->flags |= UDP_TUNNEL_NIC_ENTRY_OP_FAIL;
+}
+
+static void
+udp_tunnel_nic_device_sync_one(struct net_device *dev,
+ struct udp_tunnel_nic *utn,
+ unsigned int table, unsigned int idx)
+{
+ struct udp_tunnel_nic_table_entry *entry;
+ struct udp_tunnel_info ti;
+ int err;
+
+ entry = &utn->entries[table][idx];
+ if (!udp_tunnel_nic_entry_is_queued(entry))
+ return;
+
+ udp_tunnel_nic_ti_from_entry(entry, &ti);
+ if (entry->flags & UDP_TUNNEL_NIC_ENTRY_ADD)
+ err = dev->udp_tunnel_nic_info->set_port(dev, table, idx, &ti);
+ else
+ err = dev->udp_tunnel_nic_info->unset_port(dev, table, idx,
+ &ti);
+ udp_tunnel_nic_entry_update_done(entry, err);
+
+ if (err)
+ netdev_warn(dev,
+ "UDP tunnel port sync failed port %d type %s: %d\n",
+ be16_to_cpu(entry->port),
+ udp_tunnel_nic_tunnel_type_name(entry->type),
+ err);
+}
+
+static void
+udp_tunnel_nic_device_sync_by_port(struct net_device *dev,
+ struct udp_tunnel_nic *utn)
+{
+ const struct udp_tunnel_nic_info *info = dev->udp_tunnel_nic_info;
+ unsigned int i, j;
+
+ for (i = 0; i < utn->n_tables; i++)
+ for (j = 0; j < info->tables[i].n_entries; j++)
+ udp_tunnel_nic_device_sync_one(dev, utn, i, j);
+}
+
+static void
+udp_tunnel_nic_device_sync_by_table(struct net_device *dev,
+ struct udp_tunnel_nic *utn)
+{
+ const struct udp_tunnel_nic_info *info = dev->udp_tunnel_nic_info;
+ unsigned int i, j;
+ int err;
+
+ for (i = 0; i < utn->n_tables; i++) {
+ /* Find something that needs sync in this table */
+ for (j = 0; j < info->tables[i].n_entries; j++)
+ if (udp_tunnel_nic_entry_is_queued(&utn->entries[i][j]))
+ break;
+ if (j == info->tables[i].n_entries)
+ continue;
+
+ err = info->sync_table(dev, i);
+ if (err)
+ netdev_warn(dev, "UDP tunnel port sync failed for table %d: %d\n",
+ i, err);
+
+ for (j = 0; j < info->tables[i].n_entries; j++) {
+ struct udp_tunnel_nic_table_entry *entry;
+
+ entry = &utn->entries[i][j];
+ if (udp_tunnel_nic_entry_is_queued(entry))
+ udp_tunnel_nic_entry_update_done(entry, err);
+ }
+ }
+}
+
+static void
+__udp_tunnel_nic_device_sync(struct net_device *dev, struct udp_tunnel_nic *utn)
+{
+ if (!utn->need_sync)
+ return;
+
+ if (dev->udp_tunnel_nic_info->sync_table)
+ udp_tunnel_nic_device_sync_by_table(dev, utn);
+ else
+ udp_tunnel_nic_device_sync_by_port(dev, utn);
+
+ utn->need_sync = 0;
+ /* Can't replay directly here, in case we come from the tunnel driver's
+ * notification - trying to replay may deadlock inside tunnel driver.
+ */
+ utn->need_replay = udp_tunnel_nic_should_replay(dev, utn);
+}
+
+static void
+udp_tunnel_nic_device_sync(struct net_device *dev, struct udp_tunnel_nic *utn)
+{
+ const struct udp_tunnel_nic_info *info = dev->udp_tunnel_nic_info;
+ bool may_sleep;
+
+ if (!utn->need_sync)
+ return;
+
+ /* Drivers which sleep in the callback need to update from
+ * the workqueue, if we come from the tunnel driver's notification.
+ */
+ may_sleep = info->flags & UDP_TUNNEL_NIC_INFO_MAY_SLEEP;
+ if (!may_sleep)
+ __udp_tunnel_nic_device_sync(dev, utn);
+ if (may_sleep || utn->need_replay) {
+ queue_work(udp_tunnel_nic_workqueue, &utn->work);
+ utn->work_pending = 1;
+ }
+}
+
+static bool
+udp_tunnel_nic_table_is_capable(const struct udp_tunnel_nic_table_info *table,
+ struct udp_tunnel_info *ti)
+{
+ return table->tunnel_types & ti->type;
+}
+
+static bool
+udp_tunnel_nic_is_capable(struct net_device *dev, struct udp_tunnel_nic *utn,
+ struct udp_tunnel_info *ti)
+{
+ const struct udp_tunnel_nic_info *info = dev->udp_tunnel_nic_info;
+ unsigned int i;
+
+ /* Special case IPv4-only NICs */
+ if (info->flags & UDP_TUNNEL_NIC_INFO_IPV4_ONLY &&
+ ti->sa_family != AF_INET)
+ return false;
+
+ for (i = 0; i < utn->n_tables; i++)
+ if (udp_tunnel_nic_table_is_capable(&info->tables[i], ti))
+ return true;
+ return false;
+}
+
+static int
+udp_tunnel_nic_has_collision(struct net_device *dev, struct udp_tunnel_nic *utn,
+ struct udp_tunnel_info *ti)
+{
+ const struct udp_tunnel_nic_info *info = dev->udp_tunnel_nic_info;
+ struct udp_tunnel_nic_table_entry *entry;
+ unsigned int i, j;
+
+ for (i = 0; i < utn->n_tables; i++)
+ for (j = 0; j < info->tables[i].n_entries; j++) {
+ entry = &utn->entries[i][j];
+
+ if (!udp_tunnel_nic_entry_is_free(entry) &&
+ entry->port == ti->port &&
+ entry->type != ti->type) {
+ __set_bit(i, &utn->missed);
+ return true;
+ }
+ }
+ return false;
+}
+
+static void
+udp_tunnel_nic_entry_adj(struct udp_tunnel_nic *utn,
+ unsigned int table, unsigned int idx, int use_cnt_adj)
+{
+ struct udp_tunnel_nic_table_entry *entry = &utn->entries[table][idx];
+ bool dodgy = entry->flags & UDP_TUNNEL_NIC_ENTRY_OP_FAIL;
+ unsigned int from, to;
+
+ /* If not going from used to unused or vice versa - all done.
+ * For dodgy entries make sure we try to sync again (queue the entry).
+ */
+ entry->use_cnt += use_cnt_adj;
+ if (!dodgy && !entry->use_cnt == !(entry->use_cnt - use_cnt_adj))
+ return;
+
+ /* Cancel the op before it was sent to the device, if possible,
+ * otherwise we'd need to take special care to issue commands
+ * in the same order the ports arrived.
+ */
+ if (use_cnt_adj < 0) {
+ from = UDP_TUNNEL_NIC_ENTRY_ADD;
+ to = UDP_TUNNEL_NIC_ENTRY_DEL;
+ } else {
+ from = UDP_TUNNEL_NIC_ENTRY_DEL;
+ to = UDP_TUNNEL_NIC_ENTRY_ADD;
+ }
+
+ if (entry->flags & from) {
+ entry->flags &= ~from;
+ if (!dodgy)
+ return;
+ }
+
+ udp_tunnel_nic_entry_queue(utn, entry, to);
+}
+
+static bool
+udp_tunnel_nic_entry_try_adj(struct udp_tunnel_nic *utn,
+ unsigned int table, unsigned int idx,
+ struct udp_tunnel_info *ti, int use_cnt_adj)
+{
+ struct udp_tunnel_nic_table_entry *entry = &utn->entries[table][idx];
+
+ if (udp_tunnel_nic_entry_is_free(entry) ||
+ entry->port != ti->port ||
+ entry->type != ti->type)
+ return false;
+
+ if (udp_tunnel_nic_entry_is_frozen(entry))
+ return true;
+
+ udp_tunnel_nic_entry_adj(utn, table, idx, use_cnt_adj);
+ return true;
+}
+
+/* Try to find existing matching entry and adjust its use count, instead of
+ * adding a new one. Returns true if entry was found. In case of delete the
+ * entry may have gotten removed in the process, in which case it will be
+ * queued for removal.
+ */
+static bool
+udp_tunnel_nic_try_existing(struct net_device *dev, struct udp_tunnel_nic *utn,
+ struct udp_tunnel_info *ti, int use_cnt_adj)
+{
+ const struct udp_tunnel_nic_table_info *table;
+ unsigned int i, j;
+
+ for (i = 0; i < utn->n_tables; i++) {
+ table = &dev->udp_tunnel_nic_info->tables[i];
+ if (!udp_tunnel_nic_table_is_capable(table, ti))
+ continue;
+
+ for (j = 0; j < table->n_entries; j++)
+ if (udp_tunnel_nic_entry_try_adj(utn, i, j, ti,
+ use_cnt_adj))
+ return true;
+ }
+
+ return false;
+}
+
+static bool
+udp_tunnel_nic_add_existing(struct net_device *dev, struct udp_tunnel_nic *utn,
+ struct udp_tunnel_info *ti)
+{
+ return udp_tunnel_nic_try_existing(dev, utn, ti, +1);
+}
+
+static bool
+udp_tunnel_nic_del_existing(struct net_device *dev, struct udp_tunnel_nic *utn,
+ struct udp_tunnel_info *ti)
+{
+ return udp_tunnel_nic_try_existing(dev, utn, ti, -1);
+}
+
+static bool
+udp_tunnel_nic_add_new(struct net_device *dev, struct udp_tunnel_nic *utn,
+ struct udp_tunnel_info *ti)
+{
+ const struct udp_tunnel_nic_table_info *table;
+ unsigned int i, j;
+
+ for (i = 0; i < utn->n_tables; i++) {
+ table = &dev->udp_tunnel_nic_info->tables[i];
+ if (!udp_tunnel_nic_table_is_capable(table, ti))
+ continue;
+
+ for (j = 0; j < table->n_entries; j++) {
+ struct udp_tunnel_nic_table_entry *entry;
+
+ entry = &utn->entries[i][j];
+ if (!udp_tunnel_nic_entry_is_free(entry))
+ continue;
+
+ entry->port = ti->port;
+ entry->type = ti->type;
+ entry->use_cnt = 1;
+ udp_tunnel_nic_entry_queue(utn, entry,
+ UDP_TUNNEL_NIC_ENTRY_ADD);
+ return true;
+ }
+
+ /* The different table may still fit this port in, but there
+ * are no devices currently which have multiple tables accepting
+ * the same tunnel type, and false positives are okay.
+ */
+ __set_bit(i, &utn->missed);
+ }
+
+ return false;
+}
+
+static void
+__udp_tunnel_nic_add_port(struct net_device *dev, struct udp_tunnel_info *ti)
+{
+ const struct udp_tunnel_nic_info *info = dev->udp_tunnel_nic_info;
+ struct udp_tunnel_nic *utn;
+
+ utn = dev->udp_tunnel_nic;
+ if (!utn)
+ return;
+ if (!netif_running(dev) && info->flags & UDP_TUNNEL_NIC_INFO_OPEN_ONLY)
+ return;
+
+ if (!udp_tunnel_nic_is_capable(dev, utn, ti))
+ return;
+
+ /* It may happen that a tunnel of one type is removed and different
+ * tunnel type tries to reuse its port before the device was informed.
+ * Rely on utn->missed to re-add this port later.
+ */
+ if (udp_tunnel_nic_has_collision(dev, utn, ti))
+ return;
+
+ if (!udp_tunnel_nic_add_existing(dev, utn, ti))
+ udp_tunnel_nic_add_new(dev, utn, ti);
+
+ udp_tunnel_nic_device_sync(dev, utn);
+}
+
+static void
+__udp_tunnel_nic_del_port(struct net_device *dev, struct udp_tunnel_info *ti)
+{
+ struct udp_tunnel_nic *utn;
+
+ utn = dev->udp_tunnel_nic;
+ if (!utn)
+ return;
+
+ if (!udp_tunnel_nic_is_capable(dev, utn, ti))
+ return;
+
+ udp_tunnel_nic_del_existing(dev, utn, ti);
+
+ udp_tunnel_nic_device_sync(dev, utn);
+}
+
+static void __udp_tunnel_nic_reset_ntf(struct net_device *dev)
+{
+ const struct udp_tunnel_nic_info *info = dev->udp_tunnel_nic_info;
+ struct udp_tunnel_nic *utn;
+ unsigned int i, j;
+
+ ASSERT_RTNL();
+
+ utn = dev->udp_tunnel_nic;
+ if (!utn)
+ return;
+
+ utn->need_sync = false;
+ for (i = 0; i < utn->n_tables; i++)
+ for (j = 0; j < info->tables[i].n_entries; j++) {
+ struct udp_tunnel_nic_table_entry *entry;
+
+ entry = &utn->entries[i][j];
+
+ entry->flags &= ~(UDP_TUNNEL_NIC_ENTRY_DEL |
+ UDP_TUNNEL_NIC_ENTRY_OP_FAIL);
+ /* We don't release rtnl across ops */
+ WARN_ON(entry->flags & UDP_TUNNEL_NIC_ENTRY_FROZEN);
+ if (!entry->use_cnt)
+ continue;
+
+ udp_tunnel_nic_entry_queue(utn, entry,
+ UDP_TUNNEL_NIC_ENTRY_ADD);
+ }
+
+ __udp_tunnel_nic_device_sync(dev, utn);
+}
+
+static const struct udp_tunnel_nic_ops __udp_tunnel_nic_ops = {
+ .get_port = __udp_tunnel_nic_get_port,
+ .set_port_priv = __udp_tunnel_nic_set_port_priv,
+ .add_port = __udp_tunnel_nic_add_port,
+ .del_port = __udp_tunnel_nic_del_port,
+ .reset_ntf = __udp_tunnel_nic_reset_ntf,
+};
+
+static void
+udp_tunnel_nic_flush(struct net_device *dev, struct udp_tunnel_nic *utn)
+{
+ const struct udp_tunnel_nic_info *info = dev->udp_tunnel_nic_info;
+ unsigned int i, j;
+
+ for (i = 0; i < utn->n_tables; i++)
+ for (j = 0; j < info->tables[i].n_entries; j++) {
+ int adj_cnt = -utn->entries[i][j].use_cnt;
+
+ if (adj_cnt)
+ udp_tunnel_nic_entry_adj(utn, i, j, adj_cnt);
+ }
+
+ __udp_tunnel_nic_device_sync(dev, utn);
+
+ for (i = 0; i < utn->n_tables; i++)
+ memset(utn->entries[i], 0, array_size(info->tables[i].n_entries,
+ sizeof(**utn->entries)));
+ WARN_ON(utn->need_sync);
+ utn->need_replay = 0;
+}
+
+static void
+udp_tunnel_nic_replay(struct net_device *dev, struct udp_tunnel_nic *utn)
+{
+ const struct udp_tunnel_nic_info *info = dev->udp_tunnel_nic_info;
+ unsigned int i, j;
+
+ /* Freeze all the ports we are already tracking so that the replay
+ * does not double up the refcount.
+ */
+ for (i = 0; i < utn->n_tables; i++)
+ for (j = 0; j < info->tables[i].n_entries; j++)
+ udp_tunnel_nic_entry_freeze_used(&utn->entries[i][j]);
+ utn->missed = 0;
+ utn->need_replay = 0;
+
+ udp_tunnel_get_rx_info(dev);
+
+ for (i = 0; i < utn->n_tables; i++)
+ for (j = 0; j < info->tables[i].n_entries; j++)
+ udp_tunnel_nic_entry_unfreeze(&utn->entries[i][j]);
+}
+
+static void udp_tunnel_nic_device_sync_work(struct work_struct *work)
+{
+ struct udp_tunnel_nic *utn =
+ container_of(work, struct udp_tunnel_nic, work);
+
+ rtnl_lock();
+ utn->work_pending = 0;
+ __udp_tunnel_nic_device_sync(utn->dev, utn);
+
+ if (utn->need_replay)
+ udp_tunnel_nic_replay(utn->dev, utn);
+ rtnl_unlock();
+}
+
+static struct udp_tunnel_nic *
+udp_tunnel_nic_alloc(const struct udp_tunnel_nic_info *info,
+ unsigned int n_tables)
+{
+ struct udp_tunnel_nic *utn;
+ unsigned int i;
+
+ utn = kzalloc(sizeof(*utn), GFP_KERNEL);
+ if (!utn)
+ return NULL;
+ utn->n_tables = n_tables;
+ INIT_WORK(&utn->work, udp_tunnel_nic_device_sync_work);
+
+ utn->entries = kmalloc_array(n_tables, sizeof(void *), GFP_KERNEL);
+ if (!utn->entries)
+ goto err_free_utn;
+
+ for (i = 0; i < n_tables; i++) {
+ utn->entries[i] = kcalloc(info->tables[i].n_entries,
+ sizeof(*utn->entries[i]), GFP_KERNEL);
+ if (!utn->entries[i])
+ goto err_free_prev_entries;
+ }
+
+ return utn;
+
+err_free_prev_entries:
+ while (i--)
+ kfree(utn->entries[i]);
+ kfree(utn->entries);
+err_free_utn:
+ kfree(utn);
+ return NULL;
+}
+
+static int udp_tunnel_nic_register(struct net_device *dev)
+{
+ const struct udp_tunnel_nic_info *info = dev->udp_tunnel_nic_info;
+ struct udp_tunnel_nic *utn;
+ unsigned int n_tables, i;
+
+ BUILD_BUG_ON(sizeof(utn->missed) * BITS_PER_BYTE <
+ UDP_TUNNEL_NIC_MAX_TABLES);
+
+ if (WARN_ON(!info->set_port != !info->unset_port) ||
+ WARN_ON(!info->set_port == !info->sync_table) ||
+ WARN_ON(!info->tables[0].n_entries))
+ return -EINVAL;
+
+ n_tables = 1;
+ for (i = 1; i < UDP_TUNNEL_NIC_MAX_TABLES; i++) {
+ if (!info->tables[i].n_entries)
+ continue;
+
+ n_tables++;
+ if (WARN_ON(!info->tables[i - 1].n_entries))
+ return -EINVAL;
+ }
+
+ utn = udp_tunnel_nic_alloc(info, n_tables);
+ if (!utn)
+ return -ENOMEM;
+
+ utn->dev = dev;
+ dev_hold(dev);
+ dev->udp_tunnel_nic = utn;
+
+ if (!(info->flags & UDP_TUNNEL_NIC_INFO_OPEN_ONLY))
+ udp_tunnel_get_rx_info(dev);
+
+ return 0;
+}
+
+static void
+udp_tunnel_nic_unregister(struct net_device *dev, struct udp_tunnel_nic *utn)
+{
+ unsigned int i;
+
+ /* Flush before we check work, so we don't waste time adding entries
+ * from the work which we will boot immediately.
+ */
+ udp_tunnel_nic_flush(dev, utn);
+
+ /* Wait for the work to be done using the state, netdev core will
+ * retry unregister until we give up our reference on this device.
+ */
+ if (utn->work_pending)
+ return;
+
+ for (i = 0; i < utn->n_tables; i++)
+ kfree(utn->entries[i]);
+ kfree(utn->entries);
+ kfree(utn);
+ dev->udp_tunnel_nic = NULL;
+ dev_put(dev);
+}
+
+static int
+udp_tunnel_nic_netdevice_event(struct notifier_block *unused,
+ unsigned long event, void *ptr)
+{
+ struct net_device *dev = netdev_notifier_info_to_dev(ptr);
+ const struct udp_tunnel_nic_info *info;
+ struct udp_tunnel_nic *utn;
+
+ info = dev->udp_tunnel_nic_info;
+ if (!info)
+ return NOTIFY_DONE;
+
+ if (event == NETDEV_REGISTER) {
+ int err;
+
+ err = udp_tunnel_nic_register(dev);
+ if (err)
+ netdev_WARN(dev, "failed to register for UDP tunnel offloads: %d", err);
+ return notifier_from_errno(err);
+ }
+ /* All other events will need the udp_tunnel_nic state */
+ utn = dev->udp_tunnel_nic;
+ if (!utn)
+ return NOTIFY_DONE;
+
+ if (event == NETDEV_UNREGISTER) {
+ udp_tunnel_nic_unregister(dev, utn);
+ return NOTIFY_OK;
+ }
+
+ /* All other events only matter if NIC has to be programmed open */
+ if (!(info->flags & UDP_TUNNEL_NIC_INFO_OPEN_ONLY))
+ return NOTIFY_DONE;
+
+ if (event == NETDEV_UP) {
+ WARN_ON(!udp_tunnel_nic_is_empty(dev, utn));
+ udp_tunnel_get_rx_info(dev);
+ return NOTIFY_OK;
+ }
+ if (event == NETDEV_GOING_DOWN) {
+ udp_tunnel_nic_flush(dev, utn);
+ return NOTIFY_OK;
+ }
+
+ return NOTIFY_DONE;
+}
+
+static struct notifier_block udp_tunnel_nic_notifier_block __read_mostly = {
+ .notifier_call = udp_tunnel_nic_netdevice_event,
+};
+
+static int __init udp_tunnel_nic_init_module(void)
+{
+ int err;
+
+ udp_tunnel_nic_workqueue = alloc_workqueue("udp_tunnel_nic", 0, 0);
+ if (!udp_tunnel_nic_workqueue)
+ return -ENOMEM;
+
+ rtnl_lock();
+ udp_tunnel_nic_ops = &__udp_tunnel_nic_ops;
+ rtnl_unlock();
+
+ err = register_netdevice_notifier(&udp_tunnel_nic_notifier_block);
+ if (err)
+ goto err_unset_ops;
+
+ return 0;
+
+err_unset_ops:
+ rtnl_lock();
+ udp_tunnel_nic_ops = NULL;
+ rtnl_unlock();
+ destroy_workqueue(udp_tunnel_nic_workqueue);
+ return err;
+}
+late_initcall(udp_tunnel_nic_init_module);
+
+static void __exit udp_tunnel_nic_cleanup_module(void)
+{
+ unregister_netdevice_notifier(&udp_tunnel_nic_notifier_block);
+
+ rtnl_lock();
+ udp_tunnel_nic_ops = NULL;
+ rtnl_unlock();
+
+ destroy_workqueue(udp_tunnel_nic_workqueue);
+}
+module_exit(udp_tunnel_nic_cleanup_module);
+
+MODULE_LICENSE("GPL");
diff --git a/net/ipv4/udp_tunnel_stub.c b/net/ipv4/udp_tunnel_stub.c
new file mode 100644
index 000000000000..c4b2888f5fef
--- /dev/null
+++ b/net/ipv4/udp_tunnel_stub.c
@@ -0,0 +1,7 @@
+// SPDX-License-Identifier: GPL-2.0-only
+// Copyright (c) 2020 Facebook Inc.
+
+#include <net/udp_tunnel.h>
+
+const struct udp_tunnel_nic_ops *udp_tunnel_nic_ops;
+EXPORT_SYMBOL_GPL(udp_tunnel_nic_ops);
--
2.26.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH net-next 4/9] ethtool: add tunnel info interface
2020-07-07 21:24 [PATCH net-next 0/9] udp_tunnel: add NIC RX port offload infrastructure Jakub Kicinski
` (2 preceding siblings ...)
2020-07-07 21:24 ` [PATCH net-next 3/9] udp_tunnel: add central NIC RX port offload infrastructure Jakub Kicinski
@ 2020-07-07 21:24 ` Jakub Kicinski
2020-07-08 22:32 ` Michal Kubecek
2020-07-07 21:24 ` [PATCH net-next 5/9] netdevsim: add UDP tunnel port offload support Jakub Kicinski
` (4 subsequent siblings)
8 siblings, 1 reply; 17+ messages in thread
From: Jakub Kicinski @ 2020-07-07 21:24 UTC (permalink / raw)
To: davem
Cc: netdev, saeedm, michael.chan, edwin.peer, emil.s.tantilov,
alexander.h.duyck, jeffrey.t.kirsher, tariqt, mkubecek,
Jakub Kicinski
Add an interface to report offloaded UDP ports via ethtool netlink.
Now that core takes care of tracking which UDP tunnel ports the NICs
are aware of we can quite easily export this information out to
user space.
The responsibility of writing the netlink dumps is split between
ethtool code and udp_tunnel_nic.c - since udp_tunnel module may
not always be loaded, yet we should always report the capabilities
of the NIC.
$ ethtool --show-tunnels eth0
Tunnel information for eth0:
UDP port table 0:
Size: 4
Types: vxlan
No entries
UDP port table 1:
Size: 4
Types: geneve, vxlan-gpe
Entries (1):
port 1230, vxlan-gpe
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
Documentation/networking/ethtool-netlink.rst | 33 +++
include/net/udp_tunnel.h | 21 ++
include/uapi/linux/ethtool.h | 2 +
include/uapi/linux/ethtool_netlink.h | 55 ++++
net/ethtool/Makefile | 3 +-
net/ethtool/common.c | 9 +
net/ethtool/common.h | 1 +
net/ethtool/netlink.c | 12 +
net/ethtool/netlink.h | 4 +
net/ethtool/strset.c | 5 +
net/ethtool/tunnels.c | 261 +++++++++++++++++++
net/ipv4/udp_tunnel_nic.c | 69 +++++
12 files changed, 474 insertions(+), 1 deletion(-)
create mode 100644 net/ethtool/tunnels.c
diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst
index 396390f4936b..6a9265401d31 100644
--- a/Documentation/networking/ethtool-netlink.rst
+++ b/Documentation/networking/ethtool-netlink.rst
@@ -1230,6 +1230,39 @@ used to report the amplitude of the reflection for a given pair.
| | | ``ETHTOOL_A_CABLE_AMPLITUDE_mV`` | s16 | Reflection amplitude |
+-+-+-----------------------------------------+--------+----------------------+
+TUNNEL_INFO
+===========
+
+Gets information about the tunnel state NIC is aware of.
+
+Request contents:
+
+ ===================================== ====== ==========================
+ ``ETHTOOL_A_TUNNEL_INFO_HEADER`` nested request header
+ ===================================== ====== ==========================
+
+Kernel response contents:
+
+ +---------------------------------------------+--------+---------------------+
+ | ``ETHTOOL_A_TUNNEL_INFO_HEADER`` | nested | reply header |
+ +---------------------------------------------+--------+---------------------+
+ | ``ETHTOOL_A_TUNNEL_INFO_UDP_PORTS`` | nested | all UDP port tables |
+ +-+-------------------------------------------+--------+---------------------+
+ | | ``ETHTOOL_A_TUNNEL_UDP_TABLE`` | nested | one UDP port table |
+ +-+-+-----------------------------------------+--------+---------------------+
+ | | | ``ETHTOOL_A_TUNNEL_UDP_TABLE_SIZE`` | u32 | max size of the |
+ | | | | | table |
+ +-+-+-----------------------------------------+--------+---------------------+
+ | | | ``ETHTOOL_A_TUNNEL_UDP_TABLE_TYPES`` | u32 | bitmask of tunnel |
+ | | | | | types table can hold|
+ +-+-+-----------------------------------------+--------+---------------------+
+ | | | ``ETHTOOL_A_TUNNEL_UDP_TABLE_ENTRY`` | nested | offloaded UDP port |
+ +-+-+-+---------------------------------------+--------+---------------------+
+ | | | | ``ETHTOOL_A_TUNNEL_UDP_ENTRY_PORT`` | be16 | UDP port |
+ +-+-+-+---------------------------------------+--------+---------------------+
+ | | | | ``ETHTOOL_A_TUNNEL_UDP_ENTRY_TYPE`` | u32 | tunnel type |
+ +-+-+-+---------------------------------------+--------+---------------------+
+
Request translation
===================
diff --git a/include/net/udp_tunnel.h b/include/net/udp_tunnel.h
index 2987730577e6..f2cc2574ef9c 100644
--- a/include/net/udp_tunnel.h
+++ b/include/net/udp_tunnel.h
@@ -255,6 +255,10 @@ struct udp_tunnel_nic_ops {
void (*add_port)(struct net_device *dev, struct udp_tunnel_info *ti);
void (*del_port)(struct net_device *dev, struct udp_tunnel_info *ti);
void (*reset_ntf)(struct net_device *dev);
+
+ size_t (*dump_size)(struct net_device *dev, unsigned int table);
+ int (*dump_write)(struct net_device *dev, unsigned int table,
+ struct sk_buff *skb);
};
extern const struct udp_tunnel_nic_ops *udp_tunnel_nic_ops;
@@ -294,4 +298,21 @@ static inline void udp_tunnel_nic_reset_ntf(struct net_device *dev)
if (udp_tunnel_nic_ops)
udp_tunnel_nic_ops->reset_ntf(dev);
}
+
+static inline size_t
+udp_tunnel_nic_dump_size(struct net_device *dev, unsigned int table)
+{
+ if (!udp_tunnel_nic_ops)
+ return 0;
+ return udp_tunnel_nic_ops->dump_size(dev, table);
+}
+
+static inline int
+udp_tunnel_nic_dump_write(struct net_device *dev, unsigned int table,
+ struct sk_buff *skb)
+{
+ if (!udp_tunnel_nic_ops)
+ return 0;
+ return udp_tunnel_nic_ops->dump_write(dev, table, skb);
+}
#endif
diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index d1413538ef30..0495314ce20b 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -669,6 +669,7 @@ enum ethtool_link_ext_substate_cable_issue {
* @ETH_SS_SOF_TIMESTAMPING: SOF_TIMESTAMPING_* flags
* @ETH_SS_TS_TX_TYPES: timestamping Tx types
* @ETH_SS_TS_RX_FILTERS: timestamping Rx filters
+ * @ETH_SS_UDP_TUNNEL_TYPES: UDP tunnel types
*/
enum ethtool_stringset {
ETH_SS_TEST = 0,
@@ -686,6 +687,7 @@ enum ethtool_stringset {
ETH_SS_SOF_TIMESTAMPING,
ETH_SS_TS_TX_TYPES,
ETH_SS_TS_RX_FILTERS,
+ ETH_SS_UDP_TUNNEL_TYPES,
/* add new constants above here */
ETH_SS_COUNT
diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h
index c12ce4df4b6b..b06fc125df3f 100644
--- a/include/uapi/linux/ethtool_netlink.h
+++ b/include/uapi/linux/ethtool_netlink.h
@@ -41,6 +41,7 @@ enum {
ETHTOOL_MSG_TSINFO_GET,
ETHTOOL_MSG_CABLE_TEST_ACT,
ETHTOOL_MSG_CABLE_TEST_TDR_ACT,
+ ETHTOOL_MSG_TUNNEL_INFO_GET,
/* add new constants above here */
__ETHTOOL_MSG_USER_CNT,
@@ -556,6 +557,60 @@ enum {
ETHTOOL_A_CABLE_TEST_TDR_NTF_MAX = __ETHTOOL_A_CABLE_TEST_TDR_NTF_CNT - 1
};
+/* TUNNEL INFO */
+
+enum {
+ ETHTOOL_A_TUNNEL_INFO_UNSPEC,
+ ETHTOOL_A_TUNNEL_INFO_HEADER, /* nest - _A_HEADER_* */
+
+ ETHTOOL_A_TUNNEL_INFO_UDP_PORTS, /* nest - _UDP_TABLE */
+
+ /* add new constants above here */
+ __ETHTOOL_A_TUNNEL_INFO_CNT,
+ ETHTOOL_A_TUNNEL_INFO_MAX = (__ETHTOOL_A_TUNNEL_INFO_CNT - 1)
+};
+
+enum {
+ ETHTOOL_A_TUNNEL_UDP_UNSPEC,
+
+ ETHTOOL_A_TUNNEL_UDP_TABLE, /* nest - _UDP_TABLE_* */
+
+ /* add new constants above here */
+ __ETHTOOL_A_TUNNEL_UDP_CNT,
+ ETHTOOL_A_TUNNEL_UDP_MAX = (__ETHTOOL_A_TUNNEL_UDP_CNT - 1)
+};
+
+enum {
+ ETHTOOL_A_TUNNEL_UDP_TABLE_UNSPEC,
+
+ ETHTOOL_A_TUNNEL_UDP_TABLE_SIZE, /* u32 */
+ ETHTOOL_A_TUNNEL_UDP_TABLE_TYPES, /* u32 */
+ ETHTOOL_A_TUNNEL_UDP_TABLE_ENTRY, /* nest - _UDP_ENTRY_* */
+
+ /* add new constants above here */
+ __ETHTOOL_A_TUNNEL_UDP_TABLE_CNT,
+ ETHTOOL_A_TUNNEL_UDP_TABLE_MAX = (__ETHTOOL_A_TUNNEL_UDP_TABLE_CNT - 1)
+};
+
+enum {
+ ETHTOOL_A_TUNNEL_UDP_ENTRY_UNSPEC,
+
+ ETHTOOL_A_TUNNEL_UDP_ENTRY_PORT, /* be16 */
+ ETHTOOL_A_TUNNEL_UDP_ENTRY_TYPE, /* u32 */
+
+ /* add new constants above here */
+ __ETHTOOL_A_TUNNEL_UDP_ENTRY_CNT,
+ ETHTOOL_A_TUNNEL_UDP_ENTRY_MAX = (__ETHTOOL_A_TUNNEL_UDP_ENTRY_CNT - 1)
+};
+
+enum {
+ ETHTOOL_UDP_TUNNEL_TYPE_VXLAN,
+ ETHTOOL_UDP_TUNNEL_TYPE_GENEVE,
+ ETHTOOL_UDP_TUNNEL_TYPE_VXLAN_GPE,
+
+ __ETHTOOL_UDP_TUNNEL_TYPE_BIT_CNT
+};
+
/* generic netlink info */
#define ETHTOOL_GENL_NAME "ethtool"
#define ETHTOOL_GENL_VERSION 1
diff --git a/net/ethtool/Makefile b/net/ethtool/Makefile
index 0c2b94f20499..7a849ff22dad 100644
--- a/net/ethtool/Makefile
+++ b/net/ethtool/Makefile
@@ -6,4 +6,5 @@ obj-$(CONFIG_ETHTOOL_NETLINK) += ethtool_nl.o
ethtool_nl-y := netlink.o bitset.o strset.o linkinfo.o linkmodes.o \
linkstate.o debug.o wol.o features.o privflags.o rings.o \
- channels.o coalesce.o pause.o eee.o tsinfo.o cabletest.o
+ channels.o coalesce.o pause.o eee.o tsinfo.o cabletest.o \
+ tunnels.o
diff --git a/net/ethtool/common.c b/net/ethtool/common.c
index aaecfc916a4d..fe79f260b234 100644
--- a/net/ethtool/common.c
+++ b/net/ethtool/common.c
@@ -1,5 +1,6 @@
// SPDX-License-Identifier: GPL-2.0-only
+#include <linux/ethtool_netlink.h>
#include <linux/net_tstamp.h>
#include <linux/phy.h>
@@ -256,6 +257,14 @@ const char ts_rx_filter_names[][ETH_GSTRING_LEN] = {
};
static_assert(ARRAY_SIZE(ts_rx_filter_names) == __HWTSTAMP_FILTER_CNT);
+const char udp_tunnel_type_names[][ETH_GSTRING_LEN] = {
+ [ETHTOOL_UDP_TUNNEL_TYPE_VXLAN] = "vxlan",
+ [ETHTOOL_UDP_TUNNEL_TYPE_GENEVE] = "geneve",
+ [ETHTOOL_UDP_TUNNEL_TYPE_VXLAN_GPE] = "vxlan-gpe",
+};
+static_assert(ARRAY_SIZE(udp_tunnel_type_names) ==
+ __ETHTOOL_UDP_TUNNEL_TYPE_BIT_CNT);
+
/* return false if legacy contained non-0 deprecated fields
* maxtxpkt/maxrxpkt. rest of ksettings always updated
*/
diff --git a/net/ethtool/common.h b/net/ethtool/common.h
index a62f68ccc43a..371c69b6141e 100644
--- a/net/ethtool/common.h
+++ b/net/ethtool/common.h
@@ -28,6 +28,7 @@ extern const char wol_mode_names[][ETH_GSTRING_LEN];
extern const char sof_timestamping_names[][ETH_GSTRING_LEN];
extern const char ts_tx_type_names[][ETH_GSTRING_LEN];
extern const char ts_rx_filter_names[][ETH_GSTRING_LEN];
+extern const char udp_tunnel_type_names[][ETH_GSTRING_LEN];
int __ethtool_get_link(struct net_device *dev);
diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
index 88fd07f47040..fb9d096faaa4 100644
--- a/net/ethtool/netlink.c
+++ b/net/ethtool/netlink.c
@@ -181,6 +181,12 @@ struct sk_buff *ethnl_reply_init(size_t payload, struct net_device *dev, u8 cmd,
return NULL;
}
+void *ethnl_dump_put(struct sk_buff *skb, struct netlink_callback *cb, u8 cmd)
+{
+ return genlmsg_put(skb, NETLINK_CB(cb->skb).portid, cb->nlh->nlmsg_seq,
+ ðtool_genl_family, 0, cmd);
+}
+
void *ethnl_bcastmsg_put(struct sk_buff *skb, u8 cmd)
{
return genlmsg_put(skb, 0, ++ethnl_bcast_seq, ðtool_genl_family, 0,
@@ -849,6 +855,12 @@ static const struct genl_ops ethtool_genl_ops[] = {
.flags = GENL_UNS_ADMIN_PERM,
.doit = ethnl_act_cable_test_tdr,
},
+ {
+ .cmd = ETHTOOL_MSG_TUNNEL_INFO_GET,
+ .doit = ethnl_tunnel_info_doit,
+ .start = ethnl_tunnel_info_start,
+ .dumpit = ethnl_tunnel_info_dumpit,
+ },
};
static const struct genl_multicast_group ethtool_nl_mcgrps[] = {
diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h
index 9a96b6e90dc2..e2085005caac 100644
--- a/net/ethtool/netlink.h
+++ b/net/ethtool/netlink.h
@@ -19,6 +19,7 @@ int ethnl_fill_reply_header(struct sk_buff *skb, struct net_device *dev,
struct sk_buff *ethnl_reply_init(size_t payload, struct net_device *dev, u8 cmd,
u16 hdr_attrtype, struct genl_info *info,
void **ehdrp);
+void *ethnl_dump_put(struct sk_buff *skb, struct netlink_callback *cb, u8 cmd);
void *ethnl_bcastmsg_put(struct sk_buff *skb, u8 cmd);
int ethnl_multicast(struct sk_buff *skb, struct net_device *dev);
@@ -361,5 +362,8 @@ int ethnl_set_pause(struct sk_buff *skb, struct genl_info *info);
int ethnl_set_eee(struct sk_buff *skb, struct genl_info *info);
int ethnl_act_cable_test(struct sk_buff *skb, struct genl_info *info);
int ethnl_act_cable_test_tdr(struct sk_buff *skb, struct genl_info *info);
+int ethnl_tunnel_info_doit(struct sk_buff *skb, struct genl_info *info);
+int ethnl_tunnel_info_start(struct netlink_callback *cb);
+int ethnl_tunnel_info_dumpit(struct sk_buff *skb, struct netlink_callback *cb);
#endif /* _NET_ETHTOOL_NETLINK_H */
diff --git a/net/ethtool/strset.c b/net/ethtool/strset.c
index 0eed4e4909ab..5d3f315d4781 100644
--- a/net/ethtool/strset.c
+++ b/net/ethtool/strset.c
@@ -75,6 +75,11 @@ static const struct strset_info info_template[] = {
.count = __HWTSTAMP_FILTER_CNT,
.strings = ts_rx_filter_names,
},
+ [ETH_SS_UDP_TUNNEL_TYPES] = {
+ .per_dev = false,
+ .count = __ETHTOOL_A_TUNNEL_UDP_ENTRY_CNT,
+ .strings = udp_tunnel_type_names,
+ },
};
struct strset_req_info {
diff --git a/net/ethtool/tunnels.c b/net/ethtool/tunnels.c
new file mode 100644
index 000000000000..e9e09ea08c6a
--- /dev/null
+++ b/net/ethtool/tunnels.c
@@ -0,0 +1,261 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/ethtool_netlink.h>
+#include <net/udp_tunnel.h>
+
+#include "bitset.h"
+#include "common.h"
+#include "netlink.h"
+
+static const struct nla_policy
+ethtool_tunnel_info_policy[ETHTOOL_A_TUNNEL_INFO_MAX + 1] = {
+ [ETHTOOL_A_TUNNEL_INFO_UNSPEC] = { .type = NLA_REJECT },
+ [ETHTOOL_A_TUNNEL_INFO_HEADER] = { .type = NLA_NESTED },
+};
+
+static_assert(ETHTOOL_UDP_TUNNEL_TYPE_VXLAN == ilog2(UDP_TUNNEL_TYPE_VXLAN));
+static_assert(ETHTOOL_UDP_TUNNEL_TYPE_GENEVE == ilog2(UDP_TUNNEL_TYPE_GENEVE));
+static_assert(ETHTOOL_UDP_TUNNEL_TYPE_VXLAN_GPE ==
+ ilog2(UDP_TUNNEL_TYPE_VXLAN_GPE));
+
+static ssize_t
+ethnl_tunnel_info_reply_size(const struct ethnl_req_info *req_base,
+ struct netlink_ext_ack *extack)
+{
+ bool compact = req_base->flags & ETHTOOL_FLAG_COMPACT_BITSETS;
+ const struct udp_tunnel_nic_info *info;
+ unsigned int i;
+ size_t size;
+ int ret;
+
+ BUILD_BUG_ON(__ETHTOOL_UDP_TUNNEL_TYPE_BIT_CNT > 32);
+
+ info = req_base->dev->udp_tunnel_nic_info;
+ if (!info) {
+ NL_SET_ERR_MSG(extack,
+ "device does not report tunnel offload info");
+ return -EOPNOTSUPP;
+ }
+
+ size = nla_total_size(0); /* _INFO_UDP_PORTS */
+
+ for (i = 0; i < UDP_TUNNEL_NIC_MAX_TABLES; i++) {
+ if (!info->tables[i].n_entries)
+ return size;
+
+ size += nla_total_size(0); /* _UDP_TABLE */
+ size += nla_total_size(sizeof(u32)); /* _UDP_TABLE_SIZE */
+ ret = ethnl_bitset32_size(&info->tables[i].tunnel_types, NULL,
+ __ETHTOOL_UDP_TUNNEL_TYPE_BIT_CNT,
+ udp_tunnel_type_names, compact);
+ if (ret < 0)
+ return ret;
+ size += ret;
+
+ size += udp_tunnel_nic_dump_size(req_base->dev, i);
+ }
+
+ return size;
+}
+
+static int
+ethnl_tunnel_info_fill_reply(const struct ethnl_req_info *req_base,
+ struct sk_buff *skb)
+{
+ bool compact = req_base->flags & ETHTOOL_FLAG_COMPACT_BITSETS;
+ const struct udp_tunnel_nic_info *info;
+ struct nlattr *ports, *table;
+ unsigned int i;
+
+ info = req_base->dev->udp_tunnel_nic_info;
+ if (!info)
+ return -EOPNOTSUPP;
+
+ ports = nla_nest_start(skb, ETHTOOL_A_TUNNEL_INFO_UDP_PORTS);
+ if (!ports)
+ return -EMSGSIZE;
+
+ for (i = 0; i < UDP_TUNNEL_NIC_MAX_TABLES; i++) {
+ if (!info->tables[i].n_entries)
+ break;
+
+ table = nla_nest_start(skb, ETHTOOL_A_TUNNEL_UDP_TABLE);
+ if (!table)
+ goto err_cancel_ports;
+
+ if (nla_put_u32(skb, ETHTOOL_A_TUNNEL_UDP_TABLE_SIZE,
+ info->tables[i].n_entries))
+ goto err_cancel_table;
+
+ if (ethnl_put_bitset32(skb, ETHTOOL_A_TUNNEL_UDP_TABLE_TYPES,
+ &info->tables[i].tunnel_types, NULL,
+ __ETHTOOL_UDP_TUNNEL_TYPE_BIT_CNT,
+ udp_tunnel_type_names, compact))
+ goto err_cancel_table;
+
+ if (udp_tunnel_nic_dump_write(req_base->dev, i, skb))
+ goto err_cancel_table;
+
+ nla_nest_end(skb, table);
+ }
+
+ nla_nest_end(skb, ports);
+
+ return 0;
+
+err_cancel_table:
+ nla_nest_cancel(skb, table);
+err_cancel_ports:
+ nla_nest_cancel(skb, ports);
+ return -EMSGSIZE;
+}
+
+static int
+ethnl_tunnel_info_req_parse(struct ethnl_req_info *req_info,
+ const struct nlmsghdr *nlhdr, struct net *net,
+ struct netlink_ext_ack *extack, bool require_dev)
+{
+ struct nlattr *tb[ETHTOOL_A_TUNNEL_INFO_MAX + 1];
+ int ret;
+
+ ret = nlmsg_parse(nlhdr, GENL_HDRLEN, tb, ETHTOOL_A_TUNNEL_INFO_MAX,
+ ethtool_tunnel_info_policy, extack);
+ if (ret < 0)
+ return ret;
+
+ return ethnl_parse_header_dev_get(req_info,
+ tb[ETHTOOL_A_TUNNEL_INFO_HEADER],
+ net, extack, require_dev);
+}
+
+int ethnl_tunnel_info_doit(struct sk_buff *skb, struct genl_info *info)
+{
+ struct ethnl_req_info req_info = {};
+ struct sk_buff *rskb;
+ void *reply_payload;
+ int reply_len;
+ int ret;
+
+ ret = ethnl_tunnel_info_req_parse(&req_info, info->nlhdr,
+ genl_info_net(info), info->extack,
+ true);
+ if (ret < 0)
+ return ret;
+
+ rtnl_lock();
+ ret = ethnl_tunnel_info_reply_size(&req_info, info->extack);
+ if (ret < 0)
+ goto err_unlock_rtnl;
+ reply_len = ret + ethnl_reply_header_size();
+
+ rskb = ethnl_reply_init(reply_len, req_info.dev,
+ ETHTOOL_MSG_TUNNEL_INFO_GET,
+ ETHTOOL_A_TUNNEL_INFO_HEADER,
+ info, &reply_payload);
+ if (!rskb) {
+ ret = -ENOMEM;
+ goto err_unlock_rtnl;
+ }
+
+ ret = ethnl_tunnel_info_fill_reply(&req_info, rskb);
+ if (ret)
+ goto err_free_msg;
+ rtnl_unlock();
+ dev_put(req_info.dev);
+ genlmsg_end(rskb, reply_payload);
+
+ return genlmsg_reply(rskb, info);
+
+err_free_msg:
+ nlmsg_free(rskb);
+err_unlock_rtnl:
+ rtnl_unlock();
+ dev_put(req_info.dev);
+ return ret;
+}
+
+struct ethnl_tunnel_info_dump_ctx {
+ struct ethnl_req_info req_info;
+ int pos_hash;
+ int pos_idx;
+};
+
+int ethnl_tunnel_info_start(struct netlink_callback *cb)
+{
+ struct ethnl_tunnel_info_dump_ctx *ctx = (void *)cb->ctx;
+ int ret;
+
+ BUILD_BUG_ON(sizeof(*ctx) > sizeof(cb->ctx));
+
+ memset(ctx, 0, sizeof(*ctx));
+
+ ret = ethnl_tunnel_info_req_parse(&ctx->req_info, cb->nlh,
+ sock_net(cb->skb->sk), cb->extack,
+ false);
+ if (ctx->req_info.dev) {
+ dev_put(ctx->req_info.dev);
+ ctx->req_info.dev = NULL;
+ }
+
+ return ret;
+}
+
+int ethnl_tunnel_info_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
+{
+ struct ethnl_tunnel_info_dump_ctx *ctx = (void *)cb->ctx;
+ struct net *net = sock_net(skb->sk);
+ int s_idx = ctx->pos_idx;
+ int h, idx = 0;
+ int ret = 0;
+ void *ehdr;
+
+ rtnl_lock();
+ cb->seq = net->dev_base_seq;
+ for (h = ctx->pos_hash; h < NETDEV_HASHENTRIES; h++, s_idx = 0) {
+ struct hlist_head *head;
+ struct net_device *dev;
+
+ head = &net->dev_index_head[h];
+ idx = 0;
+ hlist_for_each_entry(dev, head, index_hlist) {
+ if (idx < s_idx)
+ goto cont;
+
+ ehdr = ethnl_dump_put(skb, cb,
+ ETHTOOL_MSG_TUNNEL_INFO_GET);
+ if (!ehdr) {
+ ret = -EMSGSIZE;
+ goto out;
+ }
+
+ ret = ethnl_fill_reply_header(skb, dev, ETHTOOL_A_TUNNEL_INFO_HEADER);
+ if (ret < 0) {
+ genlmsg_cancel(skb, ehdr);
+ goto out;
+ }
+
+ ctx->req_info.dev = dev;
+ ret = ethnl_tunnel_info_fill_reply(&ctx->req_info, skb);
+ ctx->req_info.dev = NULL;
+ if (ret < 0) {
+ genlmsg_cancel(skb, ehdr);
+ if (ret == -EOPNOTSUPP)
+ goto cont;
+ goto out;
+ }
+ genlmsg_end(skb, ehdr);
+cont:
+ idx++;
+ }
+ }
+out:
+ rtnl_unlock();
+
+ ctx->pos_hash = h;
+ ctx->pos_idx = idx;
+ nl_dump_check_consistent(cb, nlmsg_hdr(skb));
+
+ if (ret == -EMSGSIZE && skb->len)
+ return skb->len;
+ return ret;
+}
diff --git a/net/ipv4/udp_tunnel_nic.c b/net/ipv4/udp_tunnel_nic.c
index 098fb0ebe998..687c94447d5e 100644
--- a/net/ipv4/udp_tunnel_nic.c
+++ b/net/ipv4/udp_tunnel_nic.c
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: GPL-2.0-only
// Copyright (c) 2020 Facebook Inc.
+#include <linux/ethtool_netlink.h>
#include <linux/netdevice.h>
#include <linux/slab.h>
#include <linux/types.h>
@@ -72,6 +73,12 @@ udp_tunnel_nic_entry_is_free(struct udp_tunnel_nic_table_entry *entry)
return entry->use_cnt == 0 && !entry->flags;
}
+static bool
+udp_tunnel_nic_entry_is_present(struct udp_tunnel_nic_table_entry *entry)
+{
+ return entry->use_cnt && !(entry->flags & ~UDP_TUNNEL_NIC_ENTRY_FROZEN);
+}
+
static bool
udp_tunnel_nic_entry_is_frozen(struct udp_tunnel_nic_table_entry *entry)
{
@@ -571,12 +578,74 @@ static void __udp_tunnel_nic_reset_ntf(struct net_device *dev)
__udp_tunnel_nic_device_sync(dev, utn);
}
+static size_t
+__udp_tunnel_nic_dump_size(struct net_device *dev, unsigned int table)
+{
+ const struct udp_tunnel_nic_info *info = dev->udp_tunnel_nic_info;
+ struct udp_tunnel_nic *utn;
+ unsigned int j;
+ size_t size;
+
+ utn = dev->udp_tunnel_nic;
+ if (!utn)
+ return 0;
+
+ size = 0;
+ for (j = 0; j < info->tables[table].n_entries; j++) {
+ if (!udp_tunnel_nic_entry_is_present(&utn->entries[table][j]))
+ continue;
+
+ size += nla_total_size(0) + /* _TABLE_ENTRY */
+ nla_total_size(sizeof(__be16)) + /* _ENTRY_PORT */
+ nla_total_size(sizeof(u32)); /* _ENTRY_TYPE */
+ }
+
+ return size;
+}
+
+static int
+__udp_tunnel_nic_dump_write(struct net_device *dev, unsigned int table,
+ struct sk_buff *skb)
+{
+ const struct udp_tunnel_nic_info *info = dev->udp_tunnel_nic_info;
+ struct udp_tunnel_nic *utn;
+ struct nlattr *nest;
+ unsigned int j;
+
+ utn = dev->udp_tunnel_nic;
+ if (!utn)
+ return 0;
+
+ for (j = 0; j < info->tables[table].n_entries; j++) {
+ if (!udp_tunnel_nic_entry_is_present(&utn->entries[table][j]))
+ continue;
+
+ nest = nla_nest_start(skb, ETHTOOL_A_TUNNEL_UDP_TABLE_ENTRY);
+
+ if (nla_put_be16(skb, ETHTOOL_A_TUNNEL_UDP_ENTRY_PORT,
+ utn->entries[table][j].port) ||
+ nla_put_u32(skb, ETHTOOL_A_TUNNEL_UDP_ENTRY_TYPE,
+ ilog2(utn->entries[table][j].type)))
+ goto err_cancel;
+
+ nla_nest_end(skb, nest);
+ }
+
+ return 0;
+
+err_cancel:
+ nla_nest_cancel(skb, nest);
+ return -EMSGSIZE;
+}
+
static const struct udp_tunnel_nic_ops __udp_tunnel_nic_ops = {
.get_port = __udp_tunnel_nic_get_port,
.set_port_priv = __udp_tunnel_nic_set_port_priv,
.add_port = __udp_tunnel_nic_add_port,
.del_port = __udp_tunnel_nic_del_port,
.reset_ntf = __udp_tunnel_nic_reset_ntf,
+ .dump_size = __udp_tunnel_nic_dump_size,
+ .dump_write = __udp_tunnel_nic_dump_write,
};
static void
--
2.26.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH net-next 5/9] netdevsim: add UDP tunnel port offload support
2020-07-07 21:24 [PATCH net-next 0/9] udp_tunnel: add NIC RX port offload infrastructure Jakub Kicinski
` (3 preceding siblings ...)
2020-07-07 21:24 ` [PATCH net-next 4/9] ethtool: add tunnel info interface Jakub Kicinski
@ 2020-07-07 21:24 ` Jakub Kicinski
2020-07-07 21:24 ` [PATCH net-next 6/9] selftests: net: add a test for UDP tunnel info infra Jakub Kicinski
` (3 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Jakub Kicinski @ 2020-07-07 21:24 UTC (permalink / raw)
To: davem
Cc: netdev, saeedm, michael.chan, edwin.peer, emil.s.tantilov,
alexander.h.duyck, jeffrey.t.kirsher, tariqt, mkubecek,
Jakub Kicinski
Add UDP tunnel port handlers to our fake driver so we can test
the core infra.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
drivers/net/netdevsim/Makefile | 2 +-
drivers/net/netdevsim/dev.c | 1 +
drivers/net/netdevsim/netdev.c | 12 +-
drivers/net/netdevsim/netdevsim.h | 19 +++
drivers/net/netdevsim/udp_tunnels.c | 192 ++++++++++++++++++++++++++++
5 files changed, 224 insertions(+), 2 deletions(-)
create mode 100644 drivers/net/netdevsim/udp_tunnels.c
diff --git a/drivers/net/netdevsim/Makefile b/drivers/net/netdevsim/Makefile
index f4d8f62f28c2..4dfb389dbfd8 100644
--- a/drivers/net/netdevsim/Makefile
+++ b/drivers/net/netdevsim/Makefile
@@ -3,7 +3,7 @@
obj-$(CONFIG_NETDEVSIM) += netdevsim.o
netdevsim-objs := \
- netdev.o dev.o fib.o bus.o health.o
+ netdev.o dev.o fib.o bus.o health.o udp_tunnels.o
ifeq ($(CONFIG_BPF_SYSCALL),y)
netdevsim-objs += \
diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index ec6b6f7818ac..dd8f997952ca 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -225,6 +225,7 @@ static int nsim_dev_debugfs_init(struct nsim_dev *nsim_dev)
debugfs_create_bool("fail_trap_policer_counter_get", 0600,
nsim_dev->ddir,
&nsim_dev->fail_trap_policer_counter_get);
+ nsim_udp_tunnels_debugfs_create(nsim_dev);
return 0;
}
diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
index 2908e0a0d6e1..9d0d18026434 100644
--- a/drivers/net/netdevsim/netdev.c
+++ b/drivers/net/netdevsim/netdev.c
@@ -22,6 +22,7 @@
#include <net/netlink.h>
#include <net/pkt_cls.h>
#include <net/rtnetlink.h>
+#include <net/udp_tunnel.h>
#include "netdevsim.h"
@@ -257,6 +258,8 @@ static const struct net_device_ops nsim_netdev_ops = {
.ndo_setup_tc = nsim_setup_tc,
.ndo_set_features = nsim_set_features,
.ndo_bpf = nsim_bpf,
+ .ndo_udp_tunnel_add = udp_tunnel_nic_add_port,
+ .ndo_udp_tunnel_del = udp_tunnel_nic_del_port,
.ndo_get_devlink_port = nsim_get_devlink_port,
};
@@ -299,10 +302,14 @@ nsim_create(struct nsim_dev *nsim_dev, struct nsim_dev_port *nsim_dev_port)
SET_NETDEV_DEV(dev, &ns->nsim_bus_dev->dev);
dev->netdev_ops = &nsim_netdev_ops;
+ err = nsim_udp_tunnels_info_create(nsim_dev, dev);
+ if (err)
+ goto err_free_netdev;
+
rtnl_lock();
err = nsim_bpf_init(ns);
if (err)
- goto err_free_netdev;
+ goto err_utn_destroy;
nsim_ipsec_init(ns);
@@ -317,6 +324,8 @@ nsim_create(struct nsim_dev *nsim_dev, struct nsim_dev_port *nsim_dev_port)
nsim_ipsec_teardown(ns);
nsim_bpf_uninit(ns);
rtnl_unlock();
+err_utn_destroy:
+ nsim_udp_tunnels_info_destroy(dev);
err_free_netdev:
free_netdev(dev);
return ERR_PTR(err);
@@ -331,6 +340,7 @@ void nsim_destroy(struct netdevsim *ns)
nsim_ipsec_teardown(ns);
nsim_bpf_uninit(ns);
rtnl_unlock();
+ nsim_udp_tunnels_info_destroy(dev);
free_netdev(dev);
}
diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h
index 4ded54a21e1e..d164052e0393 100644
--- a/drivers/net/netdevsim/netdevsim.h
+++ b/drivers/net/netdevsim/netdevsim.h
@@ -13,6 +13,7 @@
* THE COST OF ALL NECESSARY SERVICING, REPAIR OR CORRECTION.
*/
+#include <linux/debugfs.h>
#include <linux/device.h>
#include <linux/kernel.h>
#include <linux/list.h>
@@ -29,6 +30,7 @@
#define NSIM_IPSEC_MAX_SA_COUNT 33
#define NSIM_IPSEC_VALID BIT(31)
+#define NSIM_UDP_TUNNEL_N_PORTS 4
struct nsim_sa {
struct xfrm_state *xs;
@@ -72,12 +74,23 @@ struct netdevsim {
bool bpf_map_accept;
struct nsim_ipsec ipsec;
+ struct {
+ u32 inject_error;
+ u32 sleep;
+ u32 ports[2][NSIM_UDP_TUNNEL_N_PORTS];
+ struct debugfs_u32_array dfs_ports[2];
+ } udp_ports;
};
struct netdevsim *
nsim_create(struct nsim_dev *nsim_dev, struct nsim_dev_port *nsim_dev_port);
void nsim_destroy(struct netdevsim *ns);
+void nsim_udp_tunnels_debugfs_create(struct nsim_dev *nsim_dev);
+int nsim_udp_tunnels_info_create(struct nsim_dev *nsim_dev,
+ struct net_device *dev);
+void nsim_udp_tunnels_info_destroy(struct net_device *dev);
+
#ifdef CONFIG_BPF_SYSCALL
int nsim_bpf_dev_init(struct nsim_dev *nsim_dev);
void nsim_bpf_dev_exit(struct nsim_dev *nsim_dev);
@@ -183,6 +196,12 @@ struct nsim_dev {
bool fail_trap_group_set;
bool fail_trap_policer_set;
bool fail_trap_policer_counter_get;
+ struct {
+ bool sync_all;
+ bool open_only;
+ bool ipv4_only;
+ u32 sleep;
+ } udp_ports;
};
static inline struct net *nsim_dev_net(struct nsim_dev *nsim_dev)
diff --git a/drivers/net/netdevsim/udp_tunnels.c b/drivers/net/netdevsim/udp_tunnels.c
new file mode 100644
index 000000000000..22c06a76033c
--- /dev/null
+++ b/drivers/net/netdevsim/udp_tunnels.c
@@ -0,0 +1,192 @@
+// SPDX-License-Identifier: GPL-2.0-only
+// Copyright (c) 2020 Facebook Inc.
+
+#include <linux/debugfs.h>
+#include <linux/netdevice.h>
+#include <linux/slab.h>
+#include <net/udp_tunnel.h>
+
+#include "netdevsim.h"
+
+static int
+nsim_udp_tunnel_set_port(struct net_device *dev, unsigned int table,
+ unsigned int entry, struct udp_tunnel_info *ti)
+{
+ struct netdevsim *ns = netdev_priv(dev);
+ int ret;
+
+ ret = -ns->udp_ports.inject_error;
+ ns->udp_ports.inject_error = 0;
+
+ if (ns->udp_ports.sleep)
+ msleep(ns->udp_ports.sleep);
+
+ if (!ret) {
+ if (ns->udp_ports.ports[table][entry])
+ ret = -EBUSY;
+ else
+ ns->udp_ports.ports[table][entry] =
+ be16_to_cpu(ti->port) << 16 | ti->type;
+ }
+
+ netdev_info(dev, "set [%d, %d] type %d family %d port %d - %d\n",
+ table, entry, ti->type, ti->sa_family, ntohs(ti->port),
+ ret);
+ return ret;
+}
+
+static int
+nsim_udp_tunnel_unset_port(struct net_device *dev, unsigned int table,
+ unsigned int entry, struct udp_tunnel_info *ti)
+{
+ struct netdevsim *ns = netdev_priv(dev);
+ int ret;
+
+ ret = -ns->udp_ports.inject_error;
+ ns->udp_ports.inject_error = 0;
+
+ if (ns->udp_ports.sleep)
+ msleep(ns->udp_ports.sleep);
+ if (!ret) {
+ u32 val = be16_to_cpu(ti->port) << 16 | ti->type;
+
+ if (val == ns->udp_ports.ports[table][entry])
+ ns->udp_ports.ports[table][entry] = 0;
+ else
+ ret = -ENOENT;
+ }
+
+ netdev_info(dev, "unset [%d, %d] type %d family %d port %d - %d\n",
+ table, entry, ti->type, ti->sa_family, ntohs(ti->port),
+ ret);
+ return ret;
+}
+
+static int
+nsim_udp_tunnel_sync_table(struct net_device *dev, unsigned int table)
+{
+ struct netdevsim *ns = netdev_priv(dev);
+ struct udp_tunnel_info ti;
+ unsigned int i;
+ int ret;
+
+ ret = -ns->udp_ports.inject_error;
+ ns->udp_ports.inject_error = 0;
+
+ for (i = 0; i < NSIM_UDP_TUNNEL_N_PORTS; i++) {
+ udp_tunnel_nic_get_port(dev, table, i, &ti);
+ ns->udp_ports.ports[table][i] =
+ be16_to_cpu(ti.port) << 16 | ti.type;
+ }
+
+ return ret;
+}
+
+static const struct udp_tunnel_nic_info nsim_udp_tunnel_info = {
+ .set_port = nsim_udp_tunnel_set_port,
+ .unset_port = nsim_udp_tunnel_unset_port,
+ .sync_table = nsim_udp_tunnel_sync_table,
+
+ .tables = {
+ {
+ .n_entries = NSIM_UDP_TUNNEL_N_PORTS,
+ .tunnel_types = UDP_TUNNEL_TYPE_VXLAN,
+ },
+ {
+ .n_entries = NSIM_UDP_TUNNEL_N_PORTS,
+ .tunnel_types = UDP_TUNNEL_TYPE_GENEVE |
+ UDP_TUNNEL_TYPE_VXLAN_GPE,
+ },
+ },
+};
+
+static ssize_t
+nsim_udp_tunnels_info_reset_write(struct file *file, const char __user *data,
+ size_t count, loff_t *ppos)
+{
+ struct net_device *dev = file->private_data;
+ struct netdevsim *ns = netdev_priv(dev);
+
+ memset(&ns->udp_ports.ports, 0, sizeof(ns->udp_ports.ports));
+ rtnl_lock();
+ udp_tunnel_nic_reset_ntf(dev);
+ rtnl_unlock();
+
+ return count;
+}
+
+static const struct file_operations nsim_udp_tunnels_info_reset_fops = {
+ .open = simple_open,
+ .write = nsim_udp_tunnels_info_reset_write,
+ .llseek = generic_file_llseek,
+};
+
+int nsim_udp_tunnels_info_create(struct nsim_dev *nsim_dev,
+ struct net_device *dev)
+{
+ struct netdevsim *ns = netdev_priv(dev);
+ struct udp_tunnel_nic_info *info;
+
+ debugfs_create_u32("udp_ports_inject_error", 0600,
+ ns->nsim_dev_port->ddir,
+ &ns->udp_ports.inject_error);
+
+ ns->udp_ports.dfs_ports[0].array = ns->udp_ports.ports[0];
+ ns->udp_ports.dfs_ports[0].n_elements = NSIM_UDP_TUNNEL_N_PORTS;
+ debugfs_create_u32_array("udp_ports_table0", 0400,
+ ns->nsim_dev_port->ddir,
+ &ns->udp_ports.dfs_ports[0]);
+
+ ns->udp_ports.dfs_ports[1].array = ns->udp_ports.ports[1];
+ ns->udp_ports.dfs_ports[1].n_elements = NSIM_UDP_TUNNEL_N_PORTS;
+ debugfs_create_u32_array("udp_ports_table1", 0400,
+ ns->nsim_dev_port->ddir,
+ &ns->udp_ports.dfs_ports[1]);
+
+ debugfs_create_file("udp_ports_reset", 0200, ns->nsim_dev_port->ddir,
+ dev, &nsim_udp_tunnels_info_reset_fops);
+
+ /* Note: it's not normal to allocate the info struct like this!
+ * Drivers are expected to use a static const one, here we're testing.
+ */
+ info = kmemdup(&nsim_udp_tunnel_info, sizeof(nsim_udp_tunnel_info),
+ GFP_KERNEL);
+ if (!info)
+ return -ENOMEM;
+ ns->udp_ports.sleep = nsim_dev->udp_ports.sleep;
+
+ if (nsim_dev->udp_ports.sync_all) {
+ info->set_port = NULL;
+ info->unset_port = NULL;
+ } else {
+ info->sync_table = NULL;
+ }
+
+ if (ns->udp_ports.sleep)
+ info->flags |= UDP_TUNNEL_NIC_INFO_MAY_SLEEP;
+ if (nsim_dev->udp_ports.open_only)
+ info->flags |= UDP_TUNNEL_NIC_INFO_OPEN_ONLY;
+ if (nsim_dev->udp_ports.ipv4_only)
+ info->flags |= UDP_TUNNEL_NIC_INFO_IPV4_ONLY;
+
+ dev->udp_tunnel_nic_info = info;
+ return 0;
+}
+
+void nsim_udp_tunnels_info_destroy(struct net_device *dev)
+{
+ kfree(dev->udp_tunnel_nic_info);
+ dev->udp_tunnel_nic_info = NULL;
+}
+
+void nsim_udp_tunnels_debugfs_create(struct nsim_dev *nsim_dev)
+{
+ debugfs_create_bool("udp_ports_sync_all", 0600, nsim_dev->ddir,
+ &nsim_dev->udp_ports.sync_all);
+ debugfs_create_bool("udp_ports_open_only", 0600, nsim_dev->ddir,
+ &nsim_dev->udp_ports.open_only);
+ debugfs_create_bool("udp_ports_ipv4_only", 0600, nsim_dev->ddir,
+ &nsim_dev->udp_ports.ipv4_only);
+ debugfs_create_u32("udp_ports_sleep", 0600, nsim_dev->ddir,
+ &nsim_dev->udp_ports.sleep);
+}
--
2.26.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH net-next 6/9] selftests: net: add a test for UDP tunnel info infra
2020-07-07 21:24 [PATCH net-next 0/9] udp_tunnel: add NIC RX port offload infrastructure Jakub Kicinski
` (4 preceding siblings ...)
2020-07-07 21:24 ` [PATCH net-next 5/9] netdevsim: add UDP tunnel port offload support Jakub Kicinski
@ 2020-07-07 21:24 ` Jakub Kicinski
2020-07-07 21:24 ` [PATCH net-next 7/9] ixgbe: convert to new udp_tunnel_nic infra Jakub Kicinski
` (2 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Jakub Kicinski @ 2020-07-07 21:24 UTC (permalink / raw)
To: davem
Cc: netdev, saeedm, michael.chan, edwin.peer, emil.s.tantilov,
alexander.h.duyck, jeffrey.t.kirsher, tariqt, mkubecek,
Jakub Kicinski
Add validating the UDP tunnel infra works.
$ ./udp_tunnel_nic.sh
PASSED all 383 checks
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
.../drivers/net/netdevsim/udp_tunnel_nic.sh | 786 ++++++++++++++++++
1 file changed, 786 insertions(+)
create mode 100644 tools/testing/selftests/drivers/net/netdevsim/udp_tunnel_nic.sh
diff --git a/tools/testing/selftests/drivers/net/netdevsim/udp_tunnel_nic.sh b/tools/testing/selftests/drivers/net/netdevsim/udp_tunnel_nic.sh
new file mode 100644
index 000000000000..ba1d53b9f815
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/netdevsim/udp_tunnel_nic.sh
@@ -0,0 +1,786 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0-only
+
+VNI_GEN=$RANDOM
+NSIM_ID=$((RANDOM % 1024))
+NSIM_DEV_SYS=/sys/bus/netdevsim/devices/netdevsim$NSIM_ID
+NSIM_DEV_DFS=/sys/kernel/debug/netdevsim/netdevsim$NSIM_ID
+NSIM_NETDEV=
+HAS_ETHTOOL=
+EXIT_STATUS=0
+num_cases=0
+num_errors=0
+
+clean_up_devs=( )
+
+function err_cnt {
+ echo "ERROR:" $@
+ EXIT_STATUS=1
+ ((num_errors++))
+ ((num_cases++))
+}
+
+function pass_cnt {
+ ((num_cases++))
+}
+
+function cleanup_tuns {
+ for dev in "${clean_up_devs[@]}"; do
+ [ -e /sys/class/net/$dev ] && ip link del dev $dev
+ done
+ clean_up_devs=( )
+}
+
+function cleanup_nsim {
+ if [ -e $NSIM_DEV_SYS ]; then
+ echo $NSIM_ID > /sys/bus/netdevsim/del_device
+ fi
+}
+
+function cleanup {
+ cleanup_tuns
+ cleanup_nsim
+}
+
+trap cleanup EXIT
+
+function new_vxlan {
+ local dev=$1
+ local dstport=$2
+ local lower=$3
+ local ipver=$4
+ local flags=$5
+
+ local group ipfl
+
+ [ "$ipver" != '6' ] && group=239.1.1.1 || group=fff1::1
+ [ "$ipver" != '6' ] || ipfl="-6"
+
+ [[ ! "$flags" =~ "external" ]] && flags="$flags id $((VNI_GEN++))"
+
+ ip $ipfl link add $dev type vxlan \
+ group $group \
+ dev $lower \
+ dstport $dstport \
+ $flags
+
+ ip link set dev $dev up
+
+ clean_up_devs=("${clean_up_devs[@]}" $dev)
+
+ check_tables
+}
+
+function new_geneve {
+ local dev=$1
+ local dstport=$2
+ local ipver=$3
+ local flags=$4
+
+ local group ipfl
+
+ [ "$ipver" != '6' ] && remote=1.1.1.2 || group=::2
+ [ "$ipver" != '6' ] || ipfl="-6"
+
+ [[ ! "$flags" =~ "external" ]] && flags="$flags vni $((VNI_GEN++))"
+
+ ip $ipfl link add $dev type geneve \
+ remote $remote \
+ dstport $dstport \
+ $flags
+
+ ip link set dev $dev up
+
+ clean_up_devs=("${clean_up_devs[@]}" $dev)
+
+ check_tables
+}
+
+function del_dev {
+ local dev=$1
+
+ ip link del dev $dev
+ check_tables
+}
+
+# Helpers for netdevsim port/type encoding
+function mke {
+ local port=$1
+ local type=$2
+
+ echo $((port << 16 | type))
+}
+
+function pre {
+ local val=$1
+
+ echo -e "port: $((val >> 16))\ttype: $((val & 0xffff))"
+}
+
+function pre_ethtool {
+ local val=$1
+ local port=$((val >> 16))
+ local type=$((val & 0xffff))
+
+ case $type in
+ 1)
+ type_name="vxlan"
+ ;;
+ 2)
+ type_name="geneve"
+ ;;
+ 4)
+ type_name="vxlan-gpe"
+ ;;
+ *)
+ type_name="bit X"
+ ;;
+ esac
+
+ echo "port $port, $type_name"
+}
+
+function check_table {
+ local path=$NSIM_DEV_DFS/ports/$port/udp_ports_table$1
+ local -n expected=$2
+ local last=$3
+
+ read -a have < $path
+
+ if [ ${#expected[@]} -ne ${#have[@]} ]; then
+ echo "check_table: BAD NUMBER OF ITEMS"
+ return 0
+ fi
+
+ for i in "${!expected[@]}"; do
+ if [ -n "$HAS_ETHTOOL" -a ${expected[i]} -ne 0 ]; then
+ pp_expected=`pre_ethtool ${expected[i]}`
+ ethtool --show-tunnels $NSIM_NETDEV | grep "$pp_expected" >/dev/null
+ if [ $? -ne 0 -a $last -ne 0 ]; then
+ err_cnt "ethtool table $1 on port $port: $pfx - $msg"
+ echo " check_table: ethtool does not contain '$pp_expected'"
+ ethtool --show-tunnels $NSIM_NETDEV
+ return 0
+
+ fi
+ fi
+
+ if [ ${expected[i]} != ${have[i]} ]; then
+ if [ $last -ne 0 ]; then
+ err_cnt "table $1 on port $port: $pfx - $msg"
+ echo " check_table: wrong entry $i"
+ echo " expected: `pre ${expected[i]}`"
+ echo " have: `pre ${have[i]}`"
+ return 0
+ fi
+ return 1
+ fi
+ done
+
+ pass_cnt
+ return 0
+}
+
+function check_tables {
+ # Need retries in case we have workqueue making the changes
+ local retries=10
+
+ while ! check_table 0 exp0 $((retries == 0)); do
+ sleep 0.02
+ ((retries--))
+ done
+ while ! check_table 1 exp1 $((retries == 0)); do
+ sleep 0.02
+ ((retries--))
+ done
+}
+
+function print_table {
+ local path=$NSIM_DEV_DFS/ports/$port/udp_ports_table$1
+ read -a have < $path
+
+ tree $NSIM_DEV_DFS/
+
+ echo "Port $port table $1:"
+
+ for i in "${!have[@]}"; do
+ echo " `pre ${have[i]}`"
+ done
+
+}
+
+function print_tables {
+ print_table 0
+ print_table 1
+}
+
+function get_netdev_name {
+ local -n old=$1
+
+ new=$(ls /sys/class/net)
+
+ for netdev in $new; do
+ for check in $old; do
+ [ $netdev == $check ] && break
+ done
+
+ if [ $netdev != $check ]; then
+ echo $netdev
+ break
+ fi
+ done
+}
+
+###
+### Code start
+###
+
+# Probe ethtool support
+ethtool -h | grep show-tunnels 2>&1 >/dev/null && HAS_ETHTOOL=y
+
+modprobe netdevsim
+
+# Basic test
+pfx="basic"
+
+for port in 0 1; do
+ old_netdevs=$(ls /sys/class/net)
+ if [ $port -eq 0 ]; then
+ echo $NSIM_ID > /sys/bus/netdevsim/new_device
+ else
+ echo 1 > $NSIM_DEV_DFS/udp_ports_open_only
+ echo 1 > $NSIM_DEV_DFS/udp_ports_sleep
+ echo 1 > $NSIM_DEV_SYS/new_port
+ fi
+ NSIM_NETDEV=`get_netdev_name old_netdevs`
+
+ msg="new NIC device created"
+ exp0=( 0 0 0 0 )
+ exp1=( 0 0 0 0 )
+ check_tables
+
+ msg="VxLAN v4 devices"
+ exp0=( `mke 4789 1` 0 0 0 )
+ new_vxlan vxlan0 4789 $NSIM_NETDEV
+ new_vxlan vxlan1 4789 $NSIM_NETDEV
+
+ msg="VxLAN v4 devices go down"
+ exp0=( 0 0 0 0 )
+ ifconfig vxlan1 down
+ ifconfig vxlan0 down
+ check_tables
+
+ msg="VxLAN v6 devices"
+ exp0=( `mke 4789 1` 0 0 0 )
+ new_vxlan vxlanA 4789 $NSIM_NETDEV 6
+
+ for ifc in vxlan0 vxlan1; do
+ ifconfig $ifc up
+ done
+
+ new_vxlan vxlanB 4789 $NSIM_NETDEV 6
+
+ msg="another VxLAN v6 devices"
+ exp0=( `mke 4789 1` `mke 4790 1` 0 0 )
+ new_vxlan vxlanC 4790 $NSIM_NETDEV 6
+
+ msg="Geneve device"
+ exp1=( `mke 6081 2` 0 0 0 )
+ new_geneve gnv0 6081
+
+ msg="NIC device goes down"
+ ifconfig $NSIM_NETDEV down
+ if [ $port -eq 1 ]; then
+ exp0=( 0 0 0 0 )
+ exp1=( 0 0 0 0 )
+ fi
+ check_tables
+ msg="NIC device goes up again"
+ ifconfig $NSIM_NETDEV up
+ exp0=( `mke 4789 1` `mke 4790 1` 0 0 )
+ exp1=( `mke 6081 2` 0 0 0 )
+ check_tables
+
+ cleanup_tuns
+
+ msg="tunnels destroyed"
+ exp0=( 0 0 0 0 )
+ exp1=( 0 0 0 0 )
+ check_tables
+
+ modprobe -r geneve
+ modprobe -r vxlan
+ modprobe -r udp_tunnel
+
+ check_tables
+done
+
+modprobe -r netdevsim
+
+# Module tests
+pfx="module tests"
+
+if modinfo netdevsim | grep udp_tunnel >/dev/null; then
+ err_cnt "netdevsim depends on udp_tunnel"
+else
+ pass_cnt
+fi
+
+modprobe netdevsim
+
+old_netdevs=$(ls /sys/class/net)
+port=0
+echo $NSIM_ID > /sys/bus/netdevsim/new_device
+echo 0 > $NSIM_DEV_SYS/del_port
+echo 1000 > $NSIM_DEV_DFS/udp_ports_sleep
+echo 0 > $NSIM_DEV_SYS/new_port
+NSIM_NETDEV=`get_netdev_name old_netdevs`
+
+msg="create VxLANs"
+exp0=( 0 0 0 0 ) # sleep is longer than out wait
+new_vxlan vxlan0 10000 $NSIM_NETDEV
+
+modprobe -r vxlan
+modprobe -r udp_tunnel
+
+msg="remove tunnels"
+exp0=( 0 0 0 0 )
+check_tables
+
+msg="create VxLANs"
+exp0=( 0 0 0 0 ) # sleep is longer than out wait
+new_vxlan vxlan0 10000 $NSIM_NETDEV
+
+exp0=( 0 0 0 0 )
+
+modprobe -r netdevsim
+modprobe netdevsim
+
+# Overflow the table
+
+function overflow_table0 {
+ local pfx=$1
+
+ msg="create VxLANs 1/5"
+ exp0=( `mke 10000 1` 0 0 0 )
+ new_vxlan vxlan0 10000 $NSIM_NETDEV
+
+ msg="create VxLANs 2/5"
+ exp0=( `mke 10000 1` `mke 10001 1` 0 0 )
+ new_vxlan vxlan1 10001 $NSIM_NETDEV
+
+ msg="create VxLANs 3/5"
+ exp0=( `mke 10000 1` `mke 10001 1` `mke 10002 1` 0 )
+ new_vxlan vxlan2 10002 $NSIM_NETDEV
+
+ msg="create VxLANs 4/5"
+ exp0=( `mke 10000 1` `mke 10001 1` `mke 10002 1` `mke 10003 1` )
+ new_vxlan vxlan3 10003 $NSIM_NETDEV
+
+ msg="create VxLANs 5/5"
+ new_vxlan vxlan4 10004 $NSIM_NETDEV
+}
+
+function overflow_table1 {
+ local pfx=$1
+
+ msg="create GENEVE 1/5"
+ exp1=( `mke 20000 2` 0 0 0 )
+ new_geneve gnv0 20000
+
+ msg="create GENEVE 2/5"
+ exp1=( `mke 20000 2` `mke 20001 2` 0 0 )
+ new_geneve gnv1 20001
+
+ msg="create GENEVE 3/5"
+ exp1=( `mke 20000 2` `mke 20001 2` `mke 20002 2` 0 )
+ new_geneve gnv2 20002
+
+ msg="create GENEVE 4/5"
+ exp1=( `mke 20000 2` `mke 20001 2` `mke 20002 2` `mke 20003 2` )
+ new_geneve gnv3 20003
+
+ msg="create GENEVE 5/5"
+ new_geneve gnv4 20004
+}
+
+echo $NSIM_ID > /sys/bus/netdevsim/new_device
+echo 0 > $NSIM_DEV_SYS/del_port
+
+for port in 0 1; do
+ if [ $port -ne 0 ]; then
+ echo 1 > $NSIM_DEV_DFS/udp_ports_open_only
+ echo 1 > $NSIM_DEV_DFS/udp_ports_sleep
+ fi
+
+ echo $port > $NSIM_DEV_SYS/new_port
+ ifconfig $NSIM_NETDEV up
+
+ overflow_table0 "overflow NIC table"
+ overflow_table1 "overflow NIC table"
+
+ msg="replace VxLAN in overflow table"
+ exp0=( `mke 10000 1` `mke 10004 1` `mke 10002 1` `mke 10003 1` )
+ del_dev vxlan1
+
+ msg="vacate VxLAN in overflow table"
+ exp0=( `mke 10000 1` `mke 10004 1` 0 `mke 10003 1` )
+ del_dev vxlan2
+
+ msg="replace GENEVE in overflow table"
+ exp1=( `mke 20000 2` `mke 20004 2` `mke 20002 2` `mke 20003 2` )
+ del_dev gnv1
+
+ msg="vacate GENEVE in overflow table"
+ exp1=( `mke 20000 2` `mke 20004 2` 0 `mke 20003 2` )
+ del_dev gnv2
+
+ msg="table sharing - share"
+ exp1=( `mke 20000 2` `mke 20004 2` `mke 30001 4` `mke 20003 2` )
+ new_vxlan vxlanG0 30001 $NSIM_NETDEV 4 "gpe external"
+
+ msg="table sharing - overflow"
+ new_vxlan vxlanG1 30002 $NSIM_NETDEV 4 "gpe external"
+ msg="table sharing - overflow v6"
+ new_vxlan vxlanG2 30002 $NSIM_NETDEV 6 "gpe external"
+
+ exp1=( `mke 20000 2` `mke 30002 4` `mke 30001 4` `mke 20003 2` )
+ del_dev gnv4
+
+ msg="destroy NIC"
+ echo $port > $NSIM_DEV_SYS/del_port
+
+ cleanup_tuns
+ exp0=( 0 0 0 0 )
+ exp1=( 0 0 0 0 )
+done
+
+cleanup_nsim
+
+# Sync all
+pfx="sync all"
+
+echo $NSIM_ID > /sys/bus/netdevsim/new_device
+echo 0 > $NSIM_DEV_SYS/del_port
+echo 1 > $NSIM_DEV_DFS/udp_ports_sync_all
+
+for port in 0 1; do
+ if [ $port -ne 0 ]; then
+ echo 1 > $NSIM_DEV_DFS/udp_ports_open_only
+ echo 1 > $NSIM_DEV_DFS/udp_ports_sleep
+ fi
+
+ echo $port > $NSIM_DEV_SYS/new_port
+ ifconfig $NSIM_NETDEV up
+
+ overflow_table0 "overflow NIC table"
+ overflow_table1 "overflow NIC table"
+
+ msg="replace VxLAN in overflow table"
+ exp0=( `mke 10000 1` `mke 10004 1` `mke 10002 1` `mke 10003 1` )
+ del_dev vxlan1
+
+ msg="vacate VxLAN in overflow table"
+ exp0=( `mke 10000 1` `mke 10004 1` 0 `mke 10003 1` )
+ del_dev vxlan2
+
+ msg="replace GENEVE in overflow table"
+ exp1=( `mke 20000 2` `mke 20004 2` `mke 20002 2` `mke 20003 2` )
+ del_dev gnv1
+
+ msg="vacate GENEVE in overflow table"
+ exp1=( `mke 20000 2` `mke 20004 2` 0 `mke 20003 2` )
+ del_dev gnv2
+
+ msg="table sharing - share"
+ exp1=( `mke 20000 2` `mke 20004 2` `mke 30001 4` `mke 20003 2` )
+ new_vxlan vxlanG0 30001 $NSIM_NETDEV 4 "gpe external"
+
+ msg="table sharing - overflow"
+ new_vxlan vxlanG1 30002 $NSIM_NETDEV 4 "gpe external"
+ msg="table sharing - overflow v6"
+ new_vxlan vxlanG2 30002 $NSIM_NETDEV 6 "gpe external"
+
+ exp1=( `mke 20000 2` `mke 30002 4` `mke 30001 4` `mke 20003 2` )
+ del_dev gnv4
+
+ msg="destroy NIC"
+ echo $port > $NSIM_DEV_SYS/del_port
+
+ cleanup_tuns
+ exp0=( 0 0 0 0 )
+ exp1=( 0 0 0 0 )
+done
+
+cleanup_nsim
+
+# Destroy full NIC
+pfx="destroy full"
+
+echo $NSIM_ID > /sys/bus/netdevsim/new_device
+echo 0 > $NSIM_DEV_SYS/del_port
+
+for port in 0 1; do
+ if [ $port -ne 0 ]; then
+ echo 1 > $NSIM_DEV_DFS/udp_ports_open_only
+ echo 1 > $NSIM_DEV_DFS/udp_ports_sleep
+ fi
+
+ echo $port > $NSIM_DEV_SYS/new_port
+ ifconfig $NSIM_NETDEV up
+
+ overflow_table0 "destroy NIC"
+ overflow_table1 "destroy NIC"
+
+ msg="destroy NIC"
+ echo $port > $NSIM_DEV_SYS/del_port
+
+ cleanup_tuns
+ exp0=( 0 0 0 0 )
+ exp1=( 0 0 0 0 )
+done
+
+cleanup_nsim
+
+# IPv4 only
+pfx="IPv4 only"
+
+echo $NSIM_ID > /sys/bus/netdevsim/new_device
+echo 0 > $NSIM_DEV_SYS/del_port
+echo 1 > $NSIM_DEV_DFS/udp_ports_ipv4_only
+
+for port in 0 1; do
+ if [ $port -ne 0 ]; then
+ echo 1 > $NSIM_DEV_DFS/udp_ports_open_only
+ echo 1 > $NSIM_DEV_DFS/udp_ports_sleep
+ fi
+
+ echo $port > $NSIM_DEV_SYS/new_port
+ ifconfig $NSIM_NETDEV up
+
+ msg="create VxLANs v6"
+ new_vxlan vxlanA0 10000 $NSIM_NETDEV 6
+
+ msg="create VxLANs v6"
+ new_vxlan vxlanA1 10000 $NSIM_NETDEV 6
+
+ ip link set dev vxlanA0 down
+ ip link set dev vxlanA0 up
+ check_tables
+
+ msg="create VxLANs v4"
+ exp0=( `mke 10000 1` 0 0 0 )
+ new_vxlan vxlan0 10000 $NSIM_NETDEV
+
+ msg="down VxLANs v4"
+ exp0=( 0 0 0 0 )
+ ip link set dev vxlan0 down
+ check_tables
+
+ msg="up VxLANs v4"
+ exp0=( `mke 10000 1` 0 0 0 )
+ ip link set dev vxlan0 up
+ check_tables
+
+ msg="destroy VxLANs v4"
+ exp0=( 0 0 0 0 )
+ del_dev vxlan0
+
+ msg="recreate VxLANs v4"
+ exp0=( `mke 10000 1` 0 0 0 )
+ new_vxlan vxlan0 10000 $NSIM_NETDEV
+
+ del_dev vxlanA0
+ del_dev vxlanA1
+
+ msg="destroy NIC"
+ echo $port > $NSIM_DEV_SYS/del_port
+
+ cleanup_tuns
+ exp0=( 0 0 0 0 )
+ exp1=( 0 0 0 0 )
+done
+
+cleanup_nsim
+
+# Failures
+pfx="error injection"
+
+echo $NSIM_ID > /sys/bus/netdevsim/new_device
+echo 0 > $NSIM_DEV_SYS/del_port
+
+for port in 0 1; do
+ if [ $port -ne 0 ]; then
+ echo 1 > $NSIM_DEV_DFS/udp_ports_open_only
+ echo 1 > $NSIM_DEV_DFS/udp_ports_sleep
+ fi
+
+ echo $port > $NSIM_DEV_SYS/new_port
+ ifconfig $NSIM_NETDEV up
+
+ echo 110 > $NSIM_DEV_DFS/ports/$port/udp_ports_inject_error
+
+ msg="1 - create VxLANs v6"
+ exp0=( 0 0 0 0 )
+ new_vxlan vxlanA0 10000 $NSIM_NETDEV 6
+
+ msg="1 - create VxLANs v4"
+ exp0=( `mke 10000 1` 0 0 0 )
+ new_vxlan vxlan0 10000 $NSIM_NETDEV
+
+ msg="1 - remove VxLANs v4"
+ del_dev vxlan0
+
+ msg="1 - remove VxLANs v6"
+ exp0=( 0 0 0 0 )
+ del_dev vxlanA0
+
+ msg="2 - create GENEVE"
+ exp1=( `mke 20000 2` 0 0 0 )
+ new_geneve gnv0 20000
+
+ msg="2 - destroy GENEVE"
+ echo 2 > $NSIM_DEV_DFS/ports/$port/udp_ports_inject_error
+ exp1=( `mke 20000 2` 0 0 0 )
+ del_dev gnv0
+
+ msg="2 - create second GENEVE"
+ exp1=( 0 `mke 20001 2` 0 0 )
+ new_geneve gnv0 20001
+
+ msg="destroy NIC"
+ echo $port > $NSIM_DEV_SYS/del_port
+
+ cleanup_tuns
+ exp0=( 0 0 0 0 )
+ exp1=( 0 0 0 0 )
+done
+
+cleanup_nsim
+
+# netdev flags
+pfx="netdev flags"
+
+echo $NSIM_ID > /sys/bus/netdevsim/new_device
+echo 0 > $NSIM_DEV_SYS/del_port
+
+for port in 0 1; do
+ if [ $port -ne 0 ]; then
+ echo 1 > $NSIM_DEV_DFS/udp_ports_open_only
+ echo 1 > $NSIM_DEV_DFS/udp_ports_sleep
+ fi
+
+ echo $port > $NSIM_DEV_SYS/new_port
+ ifconfig $NSIM_NETDEV up
+
+ msg="create VxLANs v6"
+ exp0=( `mke 10000 1` 0 0 0 )
+ new_vxlan vxlanA0 10000 $NSIM_NETDEV 6
+
+ msg="create VxLANs v4"
+ new_vxlan vxlan0 10000 $NSIM_NETDEV
+
+ msg="turn off"
+ exp0=( 0 0 0 0 )
+ ethtool -K $NSIM_NETDEV rx-udp_tunnel-port-offload off
+ check_tables
+
+ msg="turn on"
+ exp0=( `mke 10000 1` 0 0 0 )
+ ethtool -K $NSIM_NETDEV rx-udp_tunnel-port-offload on
+ check_tables
+
+ msg="remove both"
+ del_dev vxlanA0
+ exp0=( 0 0 0 0 )
+ del_dev vxlan0
+ check_tables
+
+ ethtool -K $NSIM_NETDEV rx-udp_tunnel-port-offload off
+
+ msg="create VxLANs v4 - off"
+ exp0=( 0 0 0 0 )
+ new_vxlan vxlan0 10000 $NSIM_NETDEV
+
+ msg="created off - turn on"
+ exp0=( `mke 10000 1` 0 0 0 )
+ ethtool -K $NSIM_NETDEV rx-udp_tunnel-port-offload on
+ check_tables
+
+ msg="destroy NIC"
+ echo $port > $NSIM_DEV_SYS/del_port
+
+ cleanup_tuns
+ exp0=( 0 0 0 0 )
+ exp1=( 0 0 0 0 )
+done
+
+cleanup_nsim
+
+# device initiated reset
+pfx="reset notification"
+
+echo $NSIM_ID > /sys/bus/netdevsim/new_device
+echo 0 > $NSIM_DEV_SYS/del_port
+
+for port in 0 1; do
+ if [ $port -ne 0 ]; then
+ echo 1 > $NSIM_DEV_DFS/udp_ports_open_only
+ echo 1 > $NSIM_DEV_DFS/udp_ports_sleep
+ fi
+
+ echo $port > $NSIM_DEV_SYS/new_port
+ ifconfig $NSIM_NETDEV up
+
+ msg="create VxLANs v6"
+ exp0=( `mke 10000 1` 0 0 0 )
+ new_vxlan vxlanA0 10000 $NSIM_NETDEV 6
+
+ msg="create VxLANs v4"
+ new_vxlan vxlan0 10000 $NSIM_NETDEV
+
+ echo 1 > $NSIM_DEV_DFS/ports/$port/udp_ports_reset
+ check_tables
+
+ msg="NIC device goes down"
+ ifconfig $NSIM_NETDEV down
+ if [ $port -eq 1 ]; then
+ exp0=( 0 0 0 0 )
+ exp1=( 0 0 0 0 )
+ fi
+ check_tables
+
+ echo 1 > $NSIM_DEV_DFS/ports/$port/udp_ports_reset
+ check_tables
+
+ msg="NIC device goes up again"
+ ifconfig $NSIM_NETDEV up
+ exp0=( `mke 10000 1` 0 0 0 )
+ check_tables
+
+ msg="remove both"
+ del_dev vxlanA0
+ exp0=( 0 0 0 0 )
+ del_dev vxlan0
+ check_tables
+
+ echo 1 > $NSIM_DEV_DFS/ports/$port/udp_ports_reset
+ check_tables
+
+ msg="destroy NIC"
+ echo $port > $NSIM_DEV_SYS/del_port
+
+ cleanup_tuns
+ exp0=( 0 0 0 0 )
+ exp1=( 0 0 0 0 )
+done
+
+modprobe -r netdevsim
+
+if [ $num_errors -eq 0 ]; then
+ echo "PASSED all $num_cases checks"
+else
+ echo "FAILED $num_errors/$num_cases checks"
+fi
+
+exit $EXIT_STATUS
--
2.26.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH net-next 7/9] ixgbe: convert to new udp_tunnel_nic infra
2020-07-07 21:24 [PATCH net-next 0/9] udp_tunnel: add NIC RX port offload infrastructure Jakub Kicinski
` (5 preceding siblings ...)
2020-07-07 21:24 ` [PATCH net-next 6/9] selftests: net: add a test for UDP tunnel info infra Jakub Kicinski
@ 2020-07-07 21:24 ` Jakub Kicinski
2020-07-08 17:00 ` Alexander Duyck
2020-07-07 21:24 ` [PATCH net-next 8/9] bnxt: " Jakub Kicinski
2020-07-07 21:24 ` [PATCH net-next 9/9] mlx4: " Jakub Kicinski
8 siblings, 1 reply; 17+ messages in thread
From: Jakub Kicinski @ 2020-07-07 21:24 UTC (permalink / raw)
To: davem
Cc: netdev, saeedm, michael.chan, edwin.peer, emil.s.tantilov,
alexander.h.duyck, jeffrey.t.kirsher, tariqt, mkubecek,
Jakub Kicinski
Make use of new common udp_tunnel_nic infra. ixgbe supports
IPv4 only, and only single VxLAN and Geneve ports (one each).
I'm dropping the confusing piece of code in ixgbe_set_features().
ndo_udp_tunnel_add and ndo_udp_tunnel_del did not check if RXCSUM
is enabled, so this code was either unnecessary or buggy anyway.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
drivers/net/ethernet/intel/ixgbe/ixgbe.h | 3 -
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 195 ++++--------------
2 files changed, 37 insertions(+), 161 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index debbcf216134..1e8a809233a0 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -588,11 +588,9 @@ struct ixgbe_adapter {
#define IXGBE_FLAG_FCOE_ENABLED BIT(21)
#define IXGBE_FLAG_SRIOV_CAPABLE BIT(22)
#define IXGBE_FLAG_SRIOV_ENABLED BIT(23)
-#define IXGBE_FLAG_VXLAN_OFFLOAD_CAPABLE BIT(24)
#define IXGBE_FLAG_RX_HWTSTAMP_ENABLED BIT(25)
#define IXGBE_FLAG_RX_HWTSTAMP_IN_REGISTER BIT(26)
#define IXGBE_FLAG_DCB_CAPABLE BIT(27)
-#define IXGBE_FLAG_GENEVE_OFFLOAD_CAPABLE BIT(28)
u32 flags2;
#define IXGBE_FLAG2_RSC_CAPABLE BIT(0)
@@ -606,7 +604,6 @@ struct ixgbe_adapter {
#define IXGBE_FLAG2_RSS_FIELD_IPV6_UDP BIT(9)
#define IXGBE_FLAG2_PTP_PPS_ENABLED BIT(10)
#define IXGBE_FLAG2_PHY_INTERRUPT BIT(11)
-#define IXGBE_FLAG2_UDP_TUN_REREG_NEEDED BIT(12)
#define IXGBE_FLAG2_VLAN_PROMISC BIT(13)
#define IXGBE_FLAG2_EEE_CAPABLE BIT(14)
#define IXGBE_FLAG2_EEE_ENABLED BIT(15)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index f5d3d6230786..29f1313b5ab0 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -4994,25 +4994,40 @@ static void ixgbe_napi_disable_all(struct ixgbe_adapter *adapter)
napi_disable(&adapter->q_vector[q_idx]->napi);
}
-static void ixgbe_clear_udp_tunnel_port(struct ixgbe_adapter *adapter, u32 mask)
+static int ixgbe_udp_tunnel_sync(struct net_device *dev, unsigned int table)
{
+ struct ixgbe_adapter *adapter = netdev_priv(dev);
struct ixgbe_hw *hw = &adapter->hw;
- u32 vxlanctrl;
-
- if (!(adapter->flags & (IXGBE_FLAG_VXLAN_OFFLOAD_CAPABLE |
- IXGBE_FLAG_GENEVE_OFFLOAD_CAPABLE)))
- return;
+ struct udp_tunnel_info ti;
- vxlanctrl = IXGBE_READ_REG(hw, IXGBE_VXLANCTRL) & ~mask;
- IXGBE_WRITE_REG(hw, IXGBE_VXLANCTRL, vxlanctrl);
-
- if (mask & IXGBE_VXLANCTRL_VXLAN_UDPPORT_MASK)
- adapter->vxlan_port = 0;
+ udp_tunnel_nic_get_port(dev, table, 0, &ti);
+ if (!table)
+ adapter->vxlan_port = ti.port;
+ else
+ adapter->geneve_port = ti.port;
- if (mask & IXGBE_VXLANCTRL_GENEVE_UDPPORT_MASK)
- adapter->geneve_port = 0;
+ IXGBE_WRITE_REG(hw, IXGBE_VXLANCTRL,
+ ntohs(adapter->vxlan_port) |
+ ntohs(adapter->geneve_port) <<
+ IXGBE_VXLANCTRL_GENEVE_UDPPORT_SHIFT);
+ return 0;
}
+static const struct udp_tunnel_nic_info ixgbe_udp_tunnels_x550 = {
+ .sync_table = ixgbe_udp_tunnel_sync,
+ .flags = UDP_TUNNEL_NIC_INFO_IPV4_ONLY,
+ .tables = {
+ { .n_entries = 1, .tunnel_types = UDP_TUNNEL_TYPE_VXLAN, },
+ },
+}, ixgbe_udp_tunnels_x550em_a = {
+ .sync_table = ixgbe_udp_tunnel_sync,
+ .flags = UDP_TUNNEL_NIC_INFO_IPV4_ONLY,
+ .tables = {
+ { .n_entries = 1, .tunnel_types = UDP_TUNNEL_TYPE_VXLAN, },
+ { .n_entries = 1, .tunnel_types = UDP_TUNNEL_TYPE_GENEVE, },
+ },
+};
+
#ifdef CONFIG_IXGBE_DCB
/**
* ixgbe_configure_dcb - Configure DCB hardware
@@ -6227,6 +6242,7 @@ static void ixgbe_init_dcb(struct ixgbe_adapter *adapter)
/**
* ixgbe_sw_init - Initialize general software structures (struct ixgbe_adapter)
* @adapter: board private structure to initialize
+ * @netdev: network interface device structure
* @ii: pointer to ixgbe_info for device
*
* ixgbe_sw_init initializes the Adapter private data structure.
@@ -6234,6 +6250,7 @@ static void ixgbe_init_dcb(struct ixgbe_adapter *adapter)
* OS network device settings (MTU size).
**/
static int ixgbe_sw_init(struct ixgbe_adapter *adapter,
+ struct net_device *netdev,
const struct ixgbe_info *ii)
{
struct ixgbe_hw *hw = &adapter->hw;
@@ -6332,7 +6349,7 @@ static int ixgbe_sw_init(struct ixgbe_adapter *adapter,
adapter->flags2 |= IXGBE_FLAG2_TEMP_SENSOR_CAPABLE;
break;
case ixgbe_mac_x550em_a:
- adapter->flags |= IXGBE_FLAG_GENEVE_OFFLOAD_CAPABLE;
+ netdev->udp_tunnel_nic_info = &ixgbe_udp_tunnels_x550em_a;
switch (hw->device_id) {
case IXGBE_DEV_ID_X550EM_A_1G_T:
case IXGBE_DEV_ID_X550EM_A_1G_T_L:
@@ -6359,7 +6376,8 @@ static int ixgbe_sw_init(struct ixgbe_adapter *adapter,
#ifdef CONFIG_IXGBE_DCA
adapter->flags &= ~IXGBE_FLAG_DCA_CAPABLE;
#endif
- adapter->flags |= IXGBE_FLAG_VXLAN_OFFLOAD_CAPABLE;
+ if (!netdev->udp_tunnel_nic_info)
+ netdev->udp_tunnel_nic_info = &ixgbe_udp_tunnels_x550;
break;
default:
break;
@@ -6798,8 +6816,7 @@ int ixgbe_open(struct net_device *netdev)
ixgbe_up_complete(adapter);
- ixgbe_clear_udp_tunnel_port(adapter, IXGBE_VXLANCTRL_ALL_UDPPORT_MASK);
- udp_tunnel_get_rx_info(netdev);
+ udp_tunnel_nic_reset_ntf(netdev);
return 0;
@@ -7921,12 +7938,6 @@ static void ixgbe_service_task(struct work_struct *work)
ixgbe_service_event_complete(adapter);
return;
}
- if (adapter->flags2 & IXGBE_FLAG2_UDP_TUN_REREG_NEEDED) {
- rtnl_lock();
- adapter->flags2 &= ~IXGBE_FLAG2_UDP_TUN_REREG_NEEDED;
- udp_tunnel_get_rx_info(adapter->netdev);
- rtnl_unlock();
- }
ixgbe_reset_subtask(adapter);
ixgbe_phy_interrupt_subtask(adapter);
ixgbe_sfp_detection_subtask(adapter);
@@ -9784,26 +9795,6 @@ static int ixgbe_set_features(struct net_device *netdev,
netdev->features = features;
- if ((adapter->flags & IXGBE_FLAG_VXLAN_OFFLOAD_CAPABLE)) {
- if (features & NETIF_F_RXCSUM) {
- adapter->flags2 |= IXGBE_FLAG2_UDP_TUN_REREG_NEEDED;
- } else {
- u32 port_mask = IXGBE_VXLANCTRL_VXLAN_UDPPORT_MASK;
-
- ixgbe_clear_udp_tunnel_port(adapter, port_mask);
- }
- }
-
- if ((adapter->flags & IXGBE_FLAG_GENEVE_OFFLOAD_CAPABLE)) {
- if (features & NETIF_F_RXCSUM) {
- adapter->flags2 |= IXGBE_FLAG2_UDP_TUN_REREG_NEEDED;
- } else {
- u32 port_mask = IXGBE_VXLANCTRL_GENEVE_UDPPORT_MASK;
-
- ixgbe_clear_udp_tunnel_port(adapter, port_mask);
- }
- }
-
if ((changed & NETIF_F_HW_L2FW_DOFFLOAD) && adapter->num_rx_pools > 1)
ixgbe_reset_l2fw_offload(adapter);
else if (need_reset)
@@ -9815,118 +9806,6 @@ static int ixgbe_set_features(struct net_device *netdev,
return 1;
}
-/**
- * ixgbe_add_udp_tunnel_port - Get notifications about adding UDP tunnel ports
- * @dev: The port's netdev
- * @ti: Tunnel endpoint information
- **/
-static void ixgbe_add_udp_tunnel_port(struct net_device *dev,
- struct udp_tunnel_info *ti)
-{
- struct ixgbe_adapter *adapter = netdev_priv(dev);
- struct ixgbe_hw *hw = &adapter->hw;
- __be16 port = ti->port;
- u32 port_shift = 0;
- u32 reg;
-
- if (ti->sa_family != AF_INET)
- return;
-
- switch (ti->type) {
- case UDP_TUNNEL_TYPE_VXLAN:
- if (!(adapter->flags & IXGBE_FLAG_VXLAN_OFFLOAD_CAPABLE))
- return;
-
- if (adapter->vxlan_port == port)
- return;
-
- if (adapter->vxlan_port) {
- netdev_info(dev,
- "VXLAN port %d set, not adding port %d\n",
- ntohs(adapter->vxlan_port),
- ntohs(port));
- return;
- }
-
- adapter->vxlan_port = port;
- break;
- case UDP_TUNNEL_TYPE_GENEVE:
- if (!(adapter->flags & IXGBE_FLAG_GENEVE_OFFLOAD_CAPABLE))
- return;
-
- if (adapter->geneve_port == port)
- return;
-
- if (adapter->geneve_port) {
- netdev_info(dev,
- "GENEVE port %d set, not adding port %d\n",
- ntohs(adapter->geneve_port),
- ntohs(port));
- return;
- }
-
- port_shift = IXGBE_VXLANCTRL_GENEVE_UDPPORT_SHIFT;
- adapter->geneve_port = port;
- break;
- default:
- return;
- }
-
- reg = IXGBE_READ_REG(hw, IXGBE_VXLANCTRL) | ntohs(port) << port_shift;
- IXGBE_WRITE_REG(hw, IXGBE_VXLANCTRL, reg);
-}
-
-/**
- * ixgbe_del_udp_tunnel_port - Get notifications about removing UDP tunnel ports
- * @dev: The port's netdev
- * @ti: Tunnel endpoint information
- **/
-static void ixgbe_del_udp_tunnel_port(struct net_device *dev,
- struct udp_tunnel_info *ti)
-{
- struct ixgbe_adapter *adapter = netdev_priv(dev);
- u32 port_mask;
-
- if (ti->type != UDP_TUNNEL_TYPE_VXLAN &&
- ti->type != UDP_TUNNEL_TYPE_GENEVE)
- return;
-
- if (ti->sa_family != AF_INET)
- return;
-
- switch (ti->type) {
- case UDP_TUNNEL_TYPE_VXLAN:
- if (!(adapter->flags & IXGBE_FLAG_VXLAN_OFFLOAD_CAPABLE))
- return;
-
- if (adapter->vxlan_port != ti->port) {
- netdev_info(dev, "VXLAN port %d not found\n",
- ntohs(ti->port));
- return;
- }
-
- port_mask = IXGBE_VXLANCTRL_VXLAN_UDPPORT_MASK;
- break;
- case UDP_TUNNEL_TYPE_GENEVE:
- if (!(adapter->flags & IXGBE_FLAG_GENEVE_OFFLOAD_CAPABLE))
- return;
-
- if (adapter->geneve_port != ti->port) {
- netdev_info(dev, "GENEVE port %d not found\n",
- ntohs(ti->port));
- return;
- }
-
- port_mask = IXGBE_VXLANCTRL_GENEVE_UDPPORT_MASK;
- break;
- default:
- return;
- }
-
- ixgbe_clear_udp_tunnel_port(adapter, port_mask);
- adapter->flags2 |= IXGBE_FLAG2_UDP_TUN_REREG_NEEDED;
-}
-
static int ixgbe_ndo_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
struct net_device *dev,
const unsigned char *addr, u16 vid,
@@ -10416,8 +10295,8 @@ static const struct net_device_ops ixgbe_netdev_ops = {
.ndo_bridge_getlink = ixgbe_ndo_bridge_getlink,
.ndo_dfwd_add_station = ixgbe_fwd_add,
.ndo_dfwd_del_station = ixgbe_fwd_del,
- .ndo_udp_tunnel_add = ixgbe_add_udp_tunnel_port,
- .ndo_udp_tunnel_del = ixgbe_del_udp_tunnel_port,
+ .ndo_udp_tunnel_add = udp_tunnel_nic_add_port,
+ .ndo_udp_tunnel_del = udp_tunnel_nic_del_port,
.ndo_features_check = ixgbe_features_check,
.ndo_bpf = ixgbe_xdp,
.ndo_xdp_xmit = ixgbe_xdp_xmit,
@@ -10858,7 +10737,7 @@ static int ixgbe_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
hw->phy.mdio.mdio_write = ixgbe_mdio_write;
/* setup the private structure */
- err = ixgbe_sw_init(adapter, ii);
+ err = ixgbe_sw_init(adapter, netdev, ii);
if (err)
goto err_sw_init;
--
2.26.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH net-next 8/9] bnxt: convert to new udp_tunnel_nic infra
2020-07-07 21:24 [PATCH net-next 0/9] udp_tunnel: add NIC RX port offload infrastructure Jakub Kicinski
` (6 preceding siblings ...)
2020-07-07 21:24 ` [PATCH net-next 7/9] ixgbe: convert to new udp_tunnel_nic infra Jakub Kicinski
@ 2020-07-07 21:24 ` Jakub Kicinski
2020-07-07 21:24 ` [PATCH net-next 9/9] mlx4: " Jakub Kicinski
8 siblings, 0 replies; 17+ messages in thread
From: Jakub Kicinski @ 2020-07-07 21:24 UTC (permalink / raw)
To: davem
Cc: netdev, saeedm, michael.chan, edwin.peer, emil.s.tantilov,
alexander.h.duyck, jeffrey.t.kirsher, tariqt, mkubecek,
Jakub Kicinski
Convert to new infra, taking advantage of sleeping in callbacks.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 133 ++++++----------------
drivers/net/ethernet/broadcom/bnxt/bnxt.h | 6 -
2 files changed, 34 insertions(+), 105 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 6a884df44612..b8c1bfeca572 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -7470,16 +7470,15 @@ static int bnxt_hwrm_pcie_qstats(struct bnxt *bp)
static void bnxt_hwrm_free_tunnel_ports(struct bnxt *bp)
{
- if (bp->vxlan_port_cnt) {
+ if (bp->vxlan_port)
bnxt_hwrm_tunnel_dst_port_free(
bp, TUNNEL_DST_PORT_FREE_REQ_TUNNEL_TYPE_VXLAN);
- }
- bp->vxlan_port_cnt = 0;
- if (bp->nge_port_cnt) {
+ bp->vxlan_port = 0;
+
+ if (bp->nge_port)
bnxt_hwrm_tunnel_dst_port_free(
bp, TUNNEL_DST_PORT_FREE_REQ_TUNNEL_TYPE_GENEVE);
- }
- bp->nge_port_cnt = 0;
+ bp->nge_port = 0;
}
static int bnxt_set_tpa(struct bnxt *bp, bool set_tpa)
@@ -9194,7 +9193,7 @@ static int __bnxt_open_nic(struct bnxt *bp, bool irq_re_init, bool link_re_init)
}
if (irq_re_init)
- udp_tunnel_get_rx_info(bp->dev);
+ udp_tunnel_nic_reset_ntf(bp->dev);
set_bit(BNXT_STATE_OPEN, &bp->state);
bnxt_enable_int(bp);
@@ -10353,24 +10352,6 @@ static void bnxt_sp_task(struct work_struct *work)
bnxt_cfg_ntp_filters(bp);
if (test_and_clear_bit(BNXT_HWRM_EXEC_FWD_REQ_SP_EVENT, &bp->sp_event))
bnxt_hwrm_exec_fwd_req(bp);
- if (test_and_clear_bit(BNXT_VXLAN_ADD_PORT_SP_EVENT, &bp->sp_event)) {
- bnxt_hwrm_tunnel_dst_port_alloc(
- bp, bp->vxlan_port,
- TUNNEL_DST_PORT_FREE_REQ_TUNNEL_TYPE_VXLAN);
- }
- if (test_and_clear_bit(BNXT_VXLAN_DEL_PORT_SP_EVENT, &bp->sp_event)) {
- bnxt_hwrm_tunnel_dst_port_free(
- bp, TUNNEL_DST_PORT_FREE_REQ_TUNNEL_TYPE_VXLAN);
- }
- if (test_and_clear_bit(BNXT_GENEVE_ADD_PORT_SP_EVENT, &bp->sp_event)) {
- bnxt_hwrm_tunnel_dst_port_alloc(
- bp, bp->nge_port,
- TUNNEL_DST_PORT_FREE_REQ_TUNNEL_TYPE_GENEVE);
- }
- if (test_and_clear_bit(BNXT_GENEVE_DEL_PORT_SP_EVENT, &bp->sp_event)) {
- bnxt_hwrm_tunnel_dst_port_free(
- bp, TUNNEL_DST_PORT_FREE_REQ_TUNNEL_TYPE_GENEVE);
- }
if (test_and_clear_bit(BNXT_PERIODIC_STATS_SP_EVENT, &bp->sp_event)) {
bnxt_hwrm_port_qstats(bp);
bnxt_hwrm_port_qstats_ext(bp);
@@ -11294,85 +11275,37 @@ static void bnxt_cfg_ntp_filters(struct bnxt *bp)
#endif /* CONFIG_RFS_ACCEL */
-static void bnxt_udp_tunnel_add(struct net_device *dev,
- struct udp_tunnel_info *ti)
+static int bnxt_udp_tunnel_sync(struct net_device *netdev, unsigned int table)
{
- struct bnxt *bp = netdev_priv(dev);
-
- if (ti->sa_family != AF_INET6 && ti->sa_family != AF_INET)
- return;
-
- if (!netif_running(dev))
- return;
-
- switch (ti->type) {
- case UDP_TUNNEL_TYPE_VXLAN:
- if (bp->vxlan_port_cnt && bp->vxlan_port != ti->port)
- return;
-
- bp->vxlan_port_cnt++;
- if (bp->vxlan_port_cnt == 1) {
- bp->vxlan_port = ti->port;
- set_bit(BNXT_VXLAN_ADD_PORT_SP_EVENT, &bp->sp_event);
- bnxt_queue_sp_work(bp);
- }
- break;
- case UDP_TUNNEL_TYPE_GENEVE:
- if (bp->nge_port_cnt && bp->nge_port != ti->port)
- return;
+ struct bnxt *bp = netdev_priv(netdev);
+ struct udp_tunnel_info ti;
+ unsigned int cmd;
- bp->nge_port_cnt++;
- if (bp->nge_port_cnt == 1) {
- bp->nge_port = ti->port;
- set_bit(BNXT_GENEVE_ADD_PORT_SP_EVENT, &bp->sp_event);
- }
- break;
- default:
- return;
+ udp_tunnel_nic_get_port(netdev, table, 0, &ti);
+ if (!table) {
+ cmd = TUNNEL_DST_PORT_FREE_REQ_TUNNEL_TYPE_VXLAN;
+ bp->vxlan_port = ti.port;
+ } else {
+ cmd = TUNNEL_DST_PORT_FREE_REQ_TUNNEL_TYPE_GENEVE;
+ bp->nge_port = ti.port;
}
- bnxt_queue_sp_work(bp);
-}
-
-static void bnxt_udp_tunnel_del(struct net_device *dev,
- struct udp_tunnel_info *ti)
-{
- struct bnxt *bp = netdev_priv(dev);
-
- if (ti->sa_family != AF_INET6 && ti->sa_family != AF_INET)
- return;
-
- if (!netif_running(dev))
- return;
-
- switch (ti->type) {
- case UDP_TUNNEL_TYPE_VXLAN:
- if (!bp->vxlan_port_cnt || bp->vxlan_port != ti->port)
- return;
- bp->vxlan_port_cnt--;
-
- if (bp->vxlan_port_cnt != 0)
- return;
-
- set_bit(BNXT_VXLAN_DEL_PORT_SP_EVENT, &bp->sp_event);
- break;
- case UDP_TUNNEL_TYPE_GENEVE:
- if (!bp->nge_port_cnt || bp->nge_port != ti->port)
- return;
- bp->nge_port_cnt--;
-
- if (bp->nge_port_cnt != 0)
- return;
-
- set_bit(BNXT_GENEVE_DEL_PORT_SP_EVENT, &bp->sp_event);
- break;
- default:
- return;
- }
+ if (ti.port)
+ return bnxt_hwrm_tunnel_dst_port_alloc(bp, ti.port, cmd);
- bnxt_queue_sp_work(bp);
+ return bnxt_hwrm_tunnel_dst_port_free(bp, cmd);
}
+static const struct udp_tunnel_nic_info bnxt_udp_tunnels = {
+ .sync_table = bnxt_udp_tunnel_sync,
+ .flags = UDP_TUNNEL_NIC_INFO_MAY_SLEEP |
+ UDP_TUNNEL_NIC_INFO_OPEN_ONLY,
+ .tables = {
+ { .n_entries = 1, .tunnel_types = UDP_TUNNEL_TYPE_VXLAN, },
+ { .n_entries = 1, .tunnel_types = UDP_TUNNEL_TYPE_GENEVE, },
+ },
+};
+
static int bnxt_bridge_getlink(struct sk_buff *skb, u32 pid, u32 seq,
struct net_device *dev, u32 filter_mask,
int nlflags)
@@ -11469,8 +11402,8 @@ static const struct net_device_ops bnxt_netdev_ops = {
#ifdef CONFIG_RFS_ACCEL
.ndo_rx_flow_steer = bnxt_rx_flow_steer,
#endif
- .ndo_udp_tunnel_add = bnxt_udp_tunnel_add,
- .ndo_udp_tunnel_del = bnxt_udp_tunnel_del,
+ .ndo_udp_tunnel_add = udp_tunnel_nic_add_port,
+ .ndo_udp_tunnel_del = udp_tunnel_nic_del_port,
.ndo_bpf = bnxt_xdp,
.ndo_xdp_xmit = bnxt_xdp_xmit,
.ndo_bridge_getlink = bnxt_bridge_getlink,
@@ -11958,6 +11891,8 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
NETIF_F_GSO_UDP_TUNNEL | NETIF_F_GSO_GRE |
NETIF_F_GSO_UDP_TUNNEL_CSUM | NETIF_F_GSO_GRE_CSUM |
NETIF_F_GSO_IPXIP4 | NETIF_F_GSO_PARTIAL;
+ dev->udp_tunnel_nic_info = &bnxt_udp_tunnels;
+
dev->gso_partial_features = NETIF_F_GSO_UDP_TUNNEL_CSUM |
NETIF_F_GSO_GRE_CSUM;
dev->vlan_features = dev->hw_features | NETIF_F_HIGHDMA;
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index 78e2fd63ac3d..352a56a18916 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -1752,10 +1752,8 @@ struct bnxt {
#define BNXT_FW_MAJ(bp) ((bp)->fw_ver_code >> 48)
__be16 vxlan_port;
- u8 vxlan_port_cnt;
__le16 vxlan_fw_dst_port_id;
__be16 nge_port;
- u8 nge_port_cnt;
__le16 nge_fw_dst_port_id;
u8 port_partition_type;
u8 port_count;
@@ -1776,16 +1774,12 @@ struct bnxt {
#define BNXT_RX_NTP_FLTR_SP_EVENT 1
#define BNXT_LINK_CHNG_SP_EVENT 2
#define BNXT_HWRM_EXEC_FWD_REQ_SP_EVENT 3
-#define BNXT_VXLAN_ADD_PORT_SP_EVENT 4
-#define BNXT_VXLAN_DEL_PORT_SP_EVENT 5
#define BNXT_RESET_TASK_SP_EVENT 6
#define BNXT_RST_RING_SP_EVENT 7
#define BNXT_HWRM_PF_UNLOAD_SP_EVENT 8
#define BNXT_PERIODIC_STATS_SP_EVENT 9
#define BNXT_HWRM_PORT_MODULE_SP_EVENT 10
#define BNXT_RESET_TASK_SILENT_SP_EVENT 11
-#define BNXT_GENEVE_ADD_PORT_SP_EVENT 12
-#define BNXT_GENEVE_DEL_PORT_SP_EVENT 13
#define BNXT_LINK_SPEED_CHNG_SP_EVENT 14
#define BNXT_FLOW_STATS_SP_EVENT 15
#define BNXT_UPDATE_PHY_SP_EVENT 16
--
2.26.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH net-next 9/9] mlx4: convert to new udp_tunnel_nic infra
2020-07-07 21:24 [PATCH net-next 0/9] udp_tunnel: add NIC RX port offload infrastructure Jakub Kicinski
` (7 preceding siblings ...)
2020-07-07 21:24 ` [PATCH net-next 8/9] bnxt: " Jakub Kicinski
@ 2020-07-07 21:24 ` Jakub Kicinski
8 siblings, 0 replies; 17+ messages in thread
From: Jakub Kicinski @ 2020-07-07 21:24 UTC (permalink / raw)
To: davem
Cc: netdev, saeedm, michael.chan, edwin.peer, emil.s.tantilov,
alexander.h.duyck, jeffrey.t.kirsher, tariqt, mkubecek,
Jakub Kicinski
Convert to new infra, make use of the ability to sleep in the callback.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
.../net/ethernet/mellanox/mlx4/en_netdev.c | 107 ++++--------------
drivers/net/ethernet/mellanox/mlx4/mlx4_en.h | 2 -
2 files changed, 25 insertions(+), 84 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index 5bd3cd37d50f..2b8608f8f0a9 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -1816,7 +1816,7 @@ int mlx4_en_start_port(struct net_device *dev)
queue_work(mdev->workqueue, &priv->rx_mode_task);
if (priv->mdev->dev->caps.tunnel_offload_mode == MLX4_TUNNEL_OFFLOAD_MODE_VXLAN)
- udp_tunnel_get_rx_info(dev);
+ udp_tunnel_nic_reset_ntf(dev);
priv->port_up = true;
@@ -2628,89 +2628,32 @@ static int mlx4_en_get_phys_port_id(struct net_device *dev,
return 0;
}
-static void mlx4_en_add_vxlan_offloads(struct work_struct *work)
+static int mlx4_udp_tunnel_sync(struct net_device *dev, unsigned int table)
{
+ struct mlx4_en_priv *priv = netdev_priv(dev);
+ struct udp_tunnel_info ti;
int ret;
- struct mlx4_en_priv *priv = container_of(work, struct mlx4_en_priv,
- vxlan_add_task);
- ret = mlx4_config_vxlan_port(priv->mdev->dev, priv->vxlan_port);
- if (ret)
- goto out;
+ udp_tunnel_nic_get_port(dev, table, 0, &ti);
+ priv->vxlan_port = ti.port;
- ret = mlx4_SET_PORT_VXLAN(priv->mdev->dev, priv->port,
- VXLAN_STEER_BY_OUTER_MAC, 1);
-out:
- if (ret) {
- en_err(priv, "failed setting L2 tunnel configuration ret %d\n", ret);
- return;
- }
-}
-
-static void mlx4_en_del_vxlan_offloads(struct work_struct *work)
-{
- int ret;
- struct mlx4_en_priv *priv = container_of(work, struct mlx4_en_priv,
- vxlan_del_task);
- ret = mlx4_SET_PORT_VXLAN(priv->mdev->dev, priv->port,
- VXLAN_STEER_BY_OUTER_MAC, 0);
+ ret = mlx4_config_vxlan_port(priv->mdev->dev, priv->vxlan_port);
if (ret)
- en_err(priv, "failed setting L2 tunnel configuration ret %d\n", ret);
+ return ret;
- priv->vxlan_port = 0;
+ return mlx4_SET_PORT_VXLAN(priv->mdev->dev, priv->port,
+ VXLAN_STEER_BY_OUTER_MAC,
+ !!priv->vxlan_port);
}
-static void mlx4_en_add_vxlan_port(struct net_device *dev,
- struct udp_tunnel_info *ti)
-{
- struct mlx4_en_priv *priv = netdev_priv(dev);
- __be16 port = ti->port;
- __be16 current_port;
-
- if (ti->type != UDP_TUNNEL_TYPE_VXLAN)
- return;
-
- if (ti->sa_family != AF_INET)
- return;
-
- if (priv->mdev->dev->caps.tunnel_offload_mode != MLX4_TUNNEL_OFFLOAD_MODE_VXLAN)
- return;
-
- current_port = priv->vxlan_port;
- if (current_port && current_port != port) {
- en_warn(priv, "vxlan port %d configured, can't add port %d\n",
- ntohs(current_port), ntohs(port));
- return;
- }
-
- priv->vxlan_port = port;
- queue_work(priv->mdev->workqueue, &priv->vxlan_add_task);
-}
-
-static void mlx4_en_del_vxlan_port(struct net_device *dev,
- struct udp_tunnel_info *ti)
-{
- struct mlx4_en_priv *priv = netdev_priv(dev);
- __be16 port = ti->port;
- __be16 current_port;
-
- if (ti->type != UDP_TUNNEL_TYPE_VXLAN)
- return;
-
- if (ti->sa_family != AF_INET)
- return;
-
- if (priv->mdev->dev->caps.tunnel_offload_mode != MLX4_TUNNEL_OFFLOAD_MODE_VXLAN)
- return;
-
- current_port = priv->vxlan_port;
- if (current_port != port) {
- en_dbg(DRV, priv, "vxlan port %d isn't configured, ignoring\n", ntohs(port));
- return;
- }
-
- queue_work(priv->mdev->workqueue, &priv->vxlan_del_task);
-}
+static const struct udp_tunnel_nic_info mlx4_udp_tunnels = {
+ .sync_table = mlx4_udp_tunnel_sync,
+ .flags = UDP_TUNNEL_NIC_INFO_MAY_SLEEP |
+ UDP_TUNNEL_NIC_INFO_IPV4_ONLY,
+ .tables = {
+ { .n_entries = 1, .tunnel_types = UDP_TUNNEL_TYPE_VXLAN, },
+ },
+};
static netdev_features_t mlx4_en_features_check(struct sk_buff *skb,
struct net_device *dev,
@@ -2914,8 +2857,8 @@ static const struct net_device_ops mlx4_netdev_ops = {
.ndo_rx_flow_steer = mlx4_en_filter_rfs,
#endif
.ndo_get_phys_port_id = mlx4_en_get_phys_port_id,
- .ndo_udp_tunnel_add = mlx4_en_add_vxlan_port,
- .ndo_udp_tunnel_del = mlx4_en_del_vxlan_port,
+ .ndo_udp_tunnel_add = udp_tunnel_nic_add_port,
+ .ndo_udp_tunnel_del = udp_tunnel_nic_del_port,
.ndo_features_check = mlx4_en_features_check,
.ndo_set_tx_maxrate = mlx4_en_set_tx_maxrate,
.ndo_bpf = mlx4_xdp,
@@ -2948,8 +2891,8 @@ static const struct net_device_ops mlx4_netdev_ops_master = {
.ndo_rx_flow_steer = mlx4_en_filter_rfs,
#endif
.ndo_get_phys_port_id = mlx4_en_get_phys_port_id,
- .ndo_udp_tunnel_add = mlx4_en_add_vxlan_port,
- .ndo_udp_tunnel_del = mlx4_en_del_vxlan_port,
+ .ndo_udp_tunnel_add = udp_tunnel_nic_add_port,
+ .ndo_udp_tunnel_del = udp_tunnel_nic_del_port,
.ndo_features_check = mlx4_en_features_check,
.ndo_set_tx_maxrate = mlx4_en_set_tx_maxrate,
.ndo_bpf = mlx4_xdp,
@@ -3250,8 +3193,6 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int port,
INIT_WORK(&priv->linkstate_task, mlx4_en_linkstate);
INIT_DELAYED_WORK(&priv->stats_task, mlx4_en_do_get_stats);
INIT_DELAYED_WORK(&priv->service_task, mlx4_en_service_task);
- INIT_WORK(&priv->vxlan_add_task, mlx4_en_add_vxlan_offloads);
- INIT_WORK(&priv->vxlan_del_task, mlx4_en_del_vxlan_offloads);
#ifdef CONFIG_RFS_ACCEL
INIT_LIST_HEAD(&priv->filters);
spin_lock_init(&priv->filters_lock);
@@ -3406,6 +3347,8 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int port,
NETIF_F_GSO_UDP_TUNNEL |
NETIF_F_GSO_UDP_TUNNEL_CSUM |
NETIF_F_GSO_PARTIAL;
+
+ dev->udp_tunnel_nic_info = &mlx4_udp_tunnels;
}
dev->vlan_features = dev->hw_features;
diff --git a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
index 9f5603612960..a46efe37cfa9 100644
--- a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
+++ b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
@@ -599,8 +599,6 @@ struct mlx4_en_priv {
struct work_struct linkstate_task;
struct delayed_work stats_task;
struct delayed_work service_task;
- struct work_struct vxlan_add_task;
- struct work_struct vxlan_del_task;
struct mlx4_en_perf_stats pstats;
struct mlx4_en_pkt_stats pkstats;
struct mlx4_en_counter_stats pf_stats;
--
2.26.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 1/9] debugfs: make sure we can remove u32_array files cleanly
2020-07-07 21:24 ` [PATCH net-next 1/9] debugfs: make sure we can remove u32_array files cleanly Jakub Kicinski
@ 2020-07-08 7:22 ` Greg Kroah-Hartman
0 siblings, 0 replies; 17+ messages in thread
From: Greg Kroah-Hartman @ 2020-07-08 7:22 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, netdev, saeedm, michael.chan, edwin.peer, emil.s.tantilov,
alexander.h.duyck, jeffrey.t.kirsher, tariqt, mkubecek,
Marek Szyprowski, Chucheng Luo
On Tue, Jul 07, 2020 at 02:24:26PM -0700, Jakub Kicinski wrote:
> debugfs_create_u32_array() allocates a small structure to wrap
> the data and size information about the array. If users ever
> try to remove the file this leads to a leak since nothing ever
> frees this wrapper.
>
> That said there are no upstream users of debugfs_create_u32_array()
> that'd remove a u32 array file (we only have one u32 array user in
> CMA), so there is no real bug here.
>
> Make callers pass a wrapper they allocated. This way the lifetime
> management of the wrapper is on the caller, and we can avoid the
> potential leak in debugfs.
>
> CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> CC: Marek Szyprowski <m.szyprowski@samsung.com>
> CC: Chucheng Luo <luochucheng@vivo.com>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 7/9] ixgbe: convert to new udp_tunnel_nic infra
2020-07-07 21:24 ` [PATCH net-next 7/9] ixgbe: convert to new udp_tunnel_nic infra Jakub Kicinski
@ 2020-07-08 17:00 ` Alexander Duyck
2020-07-08 21:25 ` Jakub Kicinski
0 siblings, 1 reply; 17+ messages in thread
From: Alexander Duyck @ 2020-07-08 17:00 UTC (permalink / raw)
To: Jakub Kicinski
Cc: David Miller, Netdev, Saeed Mahameed, Michael Chan, edwin.peer,
Tantilov, Emil S, Alexander Duyck, Jeff Kirsher, Tariq Toukan,
Michal Kubecek
On Tue, Jul 7, 2020 at 2:28 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> Make use of new common udp_tunnel_nic infra. ixgbe supports
> IPv4 only, and only single VxLAN and Geneve ports (one each).
>
> I'm dropping the confusing piece of code in ixgbe_set_features().
> ndo_udp_tunnel_add and ndo_udp_tunnel_del did not check if RXCSUM
> is enabled, so this code was either unnecessary or buggy anyway.
The code is unnecessary from what I can tell. I suspect the reason for
adding the code was because the port values are used when performing
the Rx checksum offload. However we never disable it in hardware, and
the software path in ixgbe_rx_checksum will simply disable the related
code anyway since we cannot set skb->encapsulation if RXCSUM is
disabled.
With that said moving this to a separate patch might be preferable as
it would make the patch more readable.
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
> drivers/net/ethernet/intel/ixgbe/ixgbe.h | 3 -
> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 195 ++++--------------
> 2 files changed, 37 insertions(+), 161 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> index debbcf216134..1e8a809233a0 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> @@ -588,11 +588,9 @@ struct ixgbe_adapter {
> #define IXGBE_FLAG_FCOE_ENABLED BIT(21)
> #define IXGBE_FLAG_SRIOV_CAPABLE BIT(22)
> #define IXGBE_FLAG_SRIOV_ENABLED BIT(23)
> -#define IXGBE_FLAG_VXLAN_OFFLOAD_CAPABLE BIT(24)
> #define IXGBE_FLAG_RX_HWTSTAMP_ENABLED BIT(25)
> #define IXGBE_FLAG_RX_HWTSTAMP_IN_REGISTER BIT(26)
> #define IXGBE_FLAG_DCB_CAPABLE BIT(27)
> -#define IXGBE_FLAG_GENEVE_OFFLOAD_CAPABLE BIT(28)
>
> u32 flags2;
> #define IXGBE_FLAG2_RSC_CAPABLE BIT(0)
> @@ -606,7 +604,6 @@ struct ixgbe_adapter {
> #define IXGBE_FLAG2_RSS_FIELD_IPV6_UDP BIT(9)
> #define IXGBE_FLAG2_PTP_PPS_ENABLED BIT(10)
> #define IXGBE_FLAG2_PHY_INTERRUPT BIT(11)
> -#define IXGBE_FLAG2_UDP_TUN_REREG_NEEDED BIT(12)
> #define IXGBE_FLAG2_VLAN_PROMISC BIT(13)
> #define IXGBE_FLAG2_EEE_CAPABLE BIT(14)
> #define IXGBE_FLAG2_EEE_ENABLED BIT(15)
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index f5d3d6230786..29f1313b5ab0 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -4994,25 +4994,40 @@ static void ixgbe_napi_disable_all(struct ixgbe_adapter *adapter)
> napi_disable(&adapter->q_vector[q_idx]->napi);
> }
>
> -static void ixgbe_clear_udp_tunnel_port(struct ixgbe_adapter *adapter, u32 mask)
> +static int ixgbe_udp_tunnel_sync(struct net_device *dev, unsigned int table)
> {
> + struct ixgbe_adapter *adapter = netdev_priv(dev);
> struct ixgbe_hw *hw = &adapter->hw;
> - u32 vxlanctrl;
> -
> - if (!(adapter->flags & (IXGBE_FLAG_VXLAN_OFFLOAD_CAPABLE |
> - IXGBE_FLAG_GENEVE_OFFLOAD_CAPABLE)))
> - return;
> + struct udp_tunnel_info ti;
>
> - vxlanctrl = IXGBE_READ_REG(hw, IXGBE_VXLANCTRL) & ~mask;
> - IXGBE_WRITE_REG(hw, IXGBE_VXLANCTRL, vxlanctrl);
> -
> - if (mask & IXGBE_VXLANCTRL_VXLAN_UDPPORT_MASK)
> - adapter->vxlan_port = 0;
> + udp_tunnel_nic_get_port(dev, table, 0, &ti);
> + if (!table)
> + adapter->vxlan_port = ti.port;
> + else
> + adapter->geneve_port = ti.port;
So this !table thing is a bit hard to read. It might be more useful if
you had a define that made it clear that the expectation is that entry
0 is a VXLAN table and entry 1 is the GENEVE.
>
> - if (mask & IXGBE_VXLANCTRL_GENEVE_UDPPORT_MASK)
> - adapter->geneve_port = 0;
> + IXGBE_WRITE_REG(hw, IXGBE_VXLANCTRL,
> + ntohs(adapter->vxlan_port) |
> + ntohs(adapter->geneve_port) <<
> + IXGBE_VXLANCTRL_GENEVE_UDPPORT_SHIFT);
> + return 0;
> }
I'm assuming the new logic will call this for all entries in the
tables regardless of if they are set or not. If so I suppose this is
fine.
> +static const struct udp_tunnel_nic_info ixgbe_udp_tunnels_x550 = {
> + .sync_table = ixgbe_udp_tunnel_sync,
> + .flags = UDP_TUNNEL_NIC_INFO_IPV4_ONLY,
> + .tables = {
> + { .n_entries = 1, .tunnel_types = UDP_TUNNEL_TYPE_VXLAN, },
> + },
> +}, ixgbe_udp_tunnels_x550em_a = {
> + .sync_table = ixgbe_udp_tunnel_sync,
> + .flags = UDP_TUNNEL_NIC_INFO_IPV4_ONLY,
> + .tables = {
> + { .n_entries = 1, .tunnel_types = UDP_TUNNEL_TYPE_VXLAN, },
> + { .n_entries = 1, .tunnel_types = UDP_TUNNEL_TYPE_GENEVE, },
> + },
> +};
> +
This should really be two seperate declarations. It is a bit ugly to
read a structure definition starting with "},".
> #ifdef CONFIG_IXGBE_DCB
> /**
> * ixgbe_configure_dcb - Configure DCB hardware
> @@ -6227,6 +6242,7 @@ static void ixgbe_init_dcb(struct ixgbe_adapter *adapter)
> /**
> * ixgbe_sw_init - Initialize general software structures (struct ixgbe_adapter)
> * @adapter: board private structure to initialize
> + * @netdev: network interface device structure
> * @ii: pointer to ixgbe_info for device
> *
> * ixgbe_sw_init initializes the Adapter private data structure.
> @@ -6234,6 +6250,7 @@ static void ixgbe_init_dcb(struct ixgbe_adapter *adapter)
> * OS network device settings (MTU size).
> **/
> static int ixgbe_sw_init(struct ixgbe_adapter *adapter,
> + struct net_device *netdev,
> const struct ixgbe_info *ii)
> {
> struct ixgbe_hw *hw = &adapter->hw;
There is no need to add the argument. It should be accessible via
adapter->netdev, or for that matter you could pass the netdev and drop
the adapter and just pull it out via netdev_priv since the two are all
contained in the same structure anyway. Another option would be to
just pull the logic out of this section and put it in a switch
statement of its own in the probe function.
> @@ -6332,7 +6349,7 @@ static int ixgbe_sw_init(struct ixgbe_adapter *adapter,
> adapter->flags2 |= IXGBE_FLAG2_TEMP_SENSOR_CAPABLE;
> break;
> case ixgbe_mac_x550em_a:
> - adapter->flags |= IXGBE_FLAG_GENEVE_OFFLOAD_CAPABLE;
> + netdev->udp_tunnel_nic_info = &ixgbe_udp_tunnels_x550em_a;
> switch (hw->device_id) {
> case IXGBE_DEV_ID_X550EM_A_1G_T:
> case IXGBE_DEV_ID_X550EM_A_1G_T_L:
> @@ -6359,7 +6376,8 @@ static int ixgbe_sw_init(struct ixgbe_adapter *adapter,
> #ifdef CONFIG_IXGBE_DCA
> adapter->flags &= ~IXGBE_FLAG_DCA_CAPABLE;
> #endif
> - adapter->flags |= IXGBE_FLAG_VXLAN_OFFLOAD_CAPABLE;
> + if (!netdev->udp_tunnel_nic_info)
> + netdev->udp_tunnel_nic_info = &ixgbe_udp_tunnels_x550;
> break;
> default:
> break;
Not a fan of the !udp_tunnel_nic_info check, but I understand you are
having to do it because of the fall-through.
> @@ -6798,8 +6816,7 @@ int ixgbe_open(struct net_device *netdev)
>
> ixgbe_up_complete(adapter);
>
> - ixgbe_clear_udp_tunnel_port(adapter, IXGBE_VXLANCTRL_ALL_UDPPORT_MASK);
> - udp_tunnel_get_rx_info(netdev);
> + udp_tunnel_nic_reset_ntf(netdev);
>
> return 0;
So I went looking through the earlier patches for an explanation of
udp_tunnel_nic_reset_ntf. It might be useful to have some doxygen
comments with the declaration explaining what it does and when/why we
should use it.
> @@ -7921,12 +7938,6 @@ static void ixgbe_service_task(struct work_struct *work)
> ixgbe_service_event_complete(adapter);
> return;
> }
> - if (adapter->flags2 & IXGBE_FLAG2_UDP_TUN_REREG_NEEDED) {
> - rtnl_lock();
> - adapter->flags2 &= ~IXGBE_FLAG2_UDP_TUN_REREG_NEEDED;
> - udp_tunnel_get_rx_info(adapter->netdev);
> - rtnl_unlock();
> - }
> ixgbe_reset_subtask(adapter);
> ixgbe_phy_interrupt_subtask(adapter);
> ixgbe_sfp_detection_subtask(adapter);
Like I said earlier, this and the other REREG removal bits below might
make more sense in another patch.
> @@ -9784,26 +9795,6 @@ static int ixgbe_set_features(struct net_device *netdev,
>
> netdev->features = features;
>
> - if ((adapter->flags & IXGBE_FLAG_VXLAN_OFFLOAD_CAPABLE)) {
> - if (features & NETIF_F_RXCSUM) {
> - adapter->flags2 |= IXGBE_FLAG2_UDP_TUN_REREG_NEEDED;
> - } else {
> - u32 port_mask = IXGBE_VXLANCTRL_VXLAN_UDPPORT_MASK;
> -
> - ixgbe_clear_udp_tunnel_port(adapter, port_mask);
> - }
> - }
> -
> - if ((adapter->flags & IXGBE_FLAG_GENEVE_OFFLOAD_CAPABLE)) {
> - if (features & NETIF_F_RXCSUM) {
> - adapter->flags2 |= IXGBE_FLAG2_UDP_TUN_REREG_NEEDED;
> - } else {
> - u32 port_mask = IXGBE_VXLANCTRL_GENEVE_UDPPORT_MASK;
> -
> - ixgbe_clear_udp_tunnel_port(adapter, port_mask);
> - }
> - }
> -
> if ((changed & NETIF_F_HW_L2FW_DOFFLOAD) && adapter->num_rx_pools > 1)
> ixgbe_reset_l2fw_offload(adapter);
> else if (need_reset)
> @@ -9815,118 +9806,6 @@ static int ixgbe_set_features(struct net_device *netdev,
> return 1;
> }
>
> -/**
> - * ixgbe_add_udp_tunnel_port - Get notifications about adding UDP tunnel ports
> - * @dev: The port's netdev
> - * @ti: Tunnel endpoint information
> - **/
> -static void ixgbe_add_udp_tunnel_port(struct net_device *dev,
> - struct udp_tunnel_info *ti)
> -{
> - struct ixgbe_adapter *adapter = netdev_priv(dev);
> - struct ixgbe_hw *hw = &adapter->hw;
> - __be16 port = ti->port;
> - u32 port_shift = 0;
> - u32 reg;
> -
> - if (ti->sa_family != AF_INET)
> - return;
> -
> - switch (ti->type) {
> - case UDP_TUNNEL_TYPE_VXLAN:
> - if (!(adapter->flags & IXGBE_FLAG_VXLAN_OFFLOAD_CAPABLE))
> - return;
> -
> - if (adapter->vxlan_port == port)
> - return;
> -
> - if (adapter->vxlan_port) {
> - netdev_info(dev,
> - "VXLAN port %d set, not adding port %d\n",
> - ntohs(adapter->vxlan_port),
> - ntohs(port));
> - return;
> - }
> -
> - adapter->vxlan_port = port;
> - break;
> - case UDP_TUNNEL_TYPE_GENEVE:
> - if (!(adapter->flags & IXGBE_FLAG_GENEVE_OFFLOAD_CAPABLE))
> - return;
> -
> - if (adapter->geneve_port == port)
> - return;
> -
> - if (adapter->geneve_port) {
> - netdev_info(dev,
> - "GENEVE port %d set, not adding port %d\n",
> - ntohs(adapter->geneve_port),
> - ntohs(port));
> - return;
> - }
> -
> - port_shift = IXGBE_VXLANCTRL_GENEVE_UDPPORT_SHIFT;
> - adapter->geneve_port = port;
> - break;
> - default:
> - return;
> - }
> -
> - reg = IXGBE_READ_REG(hw, IXGBE_VXLANCTRL) | ntohs(port) << port_shift;
> - IXGBE_WRITE_REG(hw, IXGBE_VXLANCTRL, reg);
> -}
> -
> -/**
> - * ixgbe_del_udp_tunnel_port - Get notifications about removing UDP tunnel ports
> - * @dev: The port's netdev
> - * @ti: Tunnel endpoint information
> - **/
> -static void ixgbe_del_udp_tunnel_port(struct net_device *dev,
> - struct udp_tunnel_info *ti)
> -{
> - struct ixgbe_adapter *adapter = netdev_priv(dev);
> - u32 port_mask;
> -
> - if (ti->type != UDP_TUNNEL_TYPE_VXLAN &&
> - ti->type != UDP_TUNNEL_TYPE_GENEVE)
> - return;
> -
> - if (ti->sa_family != AF_INET)
> - return;
> -
> - switch (ti->type) {
> - case UDP_TUNNEL_TYPE_VXLAN:
> - if (!(adapter->flags & IXGBE_FLAG_VXLAN_OFFLOAD_CAPABLE))
> - return;
> -
> - if (adapter->vxlan_port != ti->port) {
> - netdev_info(dev, "VXLAN port %d not found\n",
> - ntohs(ti->port));
> - return;
> - }
> -
> - port_mask = IXGBE_VXLANCTRL_VXLAN_UDPPORT_MASK;
> - break;
> - case UDP_TUNNEL_TYPE_GENEVE:
> - if (!(adapter->flags & IXGBE_FLAG_GENEVE_OFFLOAD_CAPABLE))
> - return;
> -
> - if (adapter->geneve_port != ti->port) {
> - netdev_info(dev, "GENEVE port %d not found\n",
> - ntohs(ti->port));
> - return;
> - }
> -
> - port_mask = IXGBE_VXLANCTRL_GENEVE_UDPPORT_MASK;
> - break;
> - default:
> - return;
> - }
> -
> - ixgbe_clear_udp_tunnel_port(adapter, port_mask);
> - adapter->flags2 |= IXGBE_FLAG2_UDP_TUN_REREG_NEEDED;
> -}
> -
> static int ixgbe_ndo_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
> struct net_device *dev,
> const unsigned char *addr, u16 vid,
> @@ -10416,8 +10295,8 @@ static const struct net_device_ops ixgbe_netdev_ops = {
> .ndo_bridge_getlink = ixgbe_ndo_bridge_getlink,
> .ndo_dfwd_add_station = ixgbe_fwd_add,
> .ndo_dfwd_del_station = ixgbe_fwd_del,
> - .ndo_udp_tunnel_add = ixgbe_add_udp_tunnel_port,
> - .ndo_udp_tunnel_del = ixgbe_del_udp_tunnel_port,
> + .ndo_udp_tunnel_add = udp_tunnel_nic_add_port,
> + .ndo_udp_tunnel_del = udp_tunnel_nic_del_port,
> .ndo_features_check = ixgbe_features_check,
> .ndo_bpf = ixgbe_xdp,
> .ndo_xdp_xmit = ixgbe_xdp_xmit,
> @@ -10858,7 +10737,7 @@ static int ixgbe_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> hw->phy.mdio.mdio_write = ixgbe_mdio_write;
>
> /* setup the private structure */
> - err = ixgbe_sw_init(adapter, ii);
> + err = ixgbe_sw_init(adapter, netdev, ii);
> if (err)
> goto err_sw_init;
>
> --
> 2.26.2
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 7/9] ixgbe: convert to new udp_tunnel_nic infra
2020-07-08 17:00 ` Alexander Duyck
@ 2020-07-08 21:25 ` Jakub Kicinski
2020-07-08 23:14 ` Alexander Duyck
0 siblings, 1 reply; 17+ messages in thread
From: Jakub Kicinski @ 2020-07-08 21:25 UTC (permalink / raw)
To: Alexander Duyck
Cc: David Miller, Netdev, Saeed Mahameed, Michael Chan, edwin.peer,
Tantilov, Emil S, Alexander Duyck, Jeff Kirsher, Tariq Toukan,
Michal Kubecek
On Wed, 8 Jul 2020 10:00:36 -0700 Alexander Duyck wrote:
> On Tue, Jul 7, 2020 at 2:28 PM Jakub Kicinski <kuba@kernel.org> wrote:
> >
> > Make use of new common udp_tunnel_nic infra. ixgbe supports
> > IPv4 only, and only single VxLAN and Geneve ports (one each).
> >
> > I'm dropping the confusing piece of code in ixgbe_set_features().
> > ndo_udp_tunnel_add and ndo_udp_tunnel_del did not check if RXCSUM
> > is enabled, so this code was either unnecessary or buggy anyway.
>
> The code is unnecessary from what I can tell. I suspect the reason for
> adding the code was because the port values are used when performing
> the Rx checksum offload. However we never disable it in hardware, and
> the software path in ixgbe_rx_checksum will simply disable the related
> code anyway since we cannot set skb->encapsulation if RXCSUM is
> disabled.
>
> With that said moving this to a separate patch might be preferable as
> it would make the patch more readable.
Ack on all points!
> > -static void ixgbe_clear_udp_tunnel_port(struct ixgbe_adapter *adapter, u32 mask)
> > +static int ixgbe_udp_tunnel_sync(struct net_device *dev, unsigned int table)
> > {
> > + struct ixgbe_adapter *adapter = netdev_priv(dev);
> > struct ixgbe_hw *hw = &adapter->hw;
> > - u32 vxlanctrl;
> > -
> > - if (!(adapter->flags & (IXGBE_FLAG_VXLAN_OFFLOAD_CAPABLE |
> > - IXGBE_FLAG_GENEVE_OFFLOAD_CAPABLE)))
> > - return;
> > + struct udp_tunnel_info ti;
> >
> > - vxlanctrl = IXGBE_READ_REG(hw, IXGBE_VXLANCTRL) & ~mask;
> > - IXGBE_WRITE_REG(hw, IXGBE_VXLANCTRL, vxlanctrl);
> > -
> > - if (mask & IXGBE_VXLANCTRL_VXLAN_UDPPORT_MASK)
> > - adapter->vxlan_port = 0;
> > + udp_tunnel_nic_get_port(dev, table, 0, &ti);
> > + if (!table)
> > + adapter->vxlan_port = ti.port;
> > + else
> > + adapter->geneve_port = ti.port;
>
> So this !table thing is a bit hard to read. It might be more useful if
> you had a define that made it clear that the expectation is that entry
> 0 is a VXLAN table and entry 1 is the GENEVE.
How about:
if (ti.type == UDP_TUNNEL_TYPE_VXLAN)
Tunnel info will have the type.
> >
> > - if (mask & IXGBE_VXLANCTRL_GENEVE_UDPPORT_MASK)
> > - adapter->geneve_port = 0;
> > + IXGBE_WRITE_REG(hw, IXGBE_VXLANCTRL,
> > + ntohs(adapter->vxlan_port) |
> > + ntohs(adapter->geneve_port) <<
> > + IXGBE_VXLANCTRL_GENEVE_UDPPORT_SHIFT);
> > + return 0;
> > }
>
> I'm assuming the new logic will call this for all entries in the
> tables regardless of if they are set or not. If so I suppose this is
> fine.
Yup, I think this is preferred form of sync for devices which just
write registers. I wrote it for NFP initially but it seems to work
in more places.
The driver can query the entire table and will get either the port
or 0 if port is not set.
> > #ifdef CONFIG_IXGBE_DCB
> > /**
> > * ixgbe_configure_dcb - Configure DCB hardware
> > @@ -6227,6 +6242,7 @@ static void ixgbe_init_dcb(struct ixgbe_adapter *adapter)
> > /**
> > * ixgbe_sw_init - Initialize general software structures (struct ixgbe_adapter)
> > * @adapter: board private structure to initialize
> > + * @netdev: network interface device structure
> > * @ii: pointer to ixgbe_info for device
> > *
> > * ixgbe_sw_init initializes the Adapter private data structure.
> > @@ -6234,6 +6250,7 @@ static void ixgbe_init_dcb(struct ixgbe_adapter *adapter)
> > * OS network device settings (MTU size).
> > **/
> > static int ixgbe_sw_init(struct ixgbe_adapter *adapter,
> > + struct net_device *netdev,
> > const struct ixgbe_info *ii)
> > {
> > struct ixgbe_hw *hw = &adapter->hw;
>
> There is no need to add the argument. It should be accessible via
> adapter->netdev, or for that matter you could pass the netdev and drop
> the adapter and just pull it out via netdev_priv since the two are all
> contained in the same structure anyway. Another option would be to
> just pull the logic out of this section and put it in a switch
> statement of its own in the probe function.
Hm, new switch section definitely looks best. I'll do that.
> > @@ -6332,7 +6349,7 @@ static int ixgbe_sw_init(struct ixgbe_adapter *adapter,
> > adapter->flags2 |= IXGBE_FLAG2_TEMP_SENSOR_CAPABLE;
> > break;
> > case ixgbe_mac_x550em_a:
> > - adapter->flags |= IXGBE_FLAG_GENEVE_OFFLOAD_CAPABLE;
> > + netdev->udp_tunnel_nic_info = &ixgbe_udp_tunnels_x550em_a;
> > switch (hw->device_id) {
> > case IXGBE_DEV_ID_X550EM_A_1G_T:
> > case IXGBE_DEV_ID_X550EM_A_1G_T_L:
> > @@ -6359,7 +6376,8 @@ static int ixgbe_sw_init(struct ixgbe_adapter *adapter,
> > #ifdef CONFIG_IXGBE_DCA
> > adapter->flags &= ~IXGBE_FLAG_DCA_CAPABLE;
> > #endif
> > - adapter->flags |= IXGBE_FLAG_VXLAN_OFFLOAD_CAPABLE;
> > + if (!netdev->udp_tunnel_nic_info)
> > + netdev->udp_tunnel_nic_info = &ixgbe_udp_tunnels_x550;
> > break;
> > default:
> > break;
>
> Not a fan of the !udp_tunnel_nic_info check, but I understand you are
> having to do it because of the fall-through.
Yup.
> > @@ -6798,8 +6816,7 @@ int ixgbe_open(struct net_device *netdev)
> >
> > ixgbe_up_complete(adapter);
> >
> > - ixgbe_clear_udp_tunnel_port(adapter, IXGBE_VXLANCTRL_ALL_UDPPORT_MASK);
> > - udp_tunnel_get_rx_info(netdev);
> > + udp_tunnel_nic_reset_ntf(netdev);
> >
> > return 0;
>
> So I went looking through the earlier patches for an explanation of
> udp_tunnel_nic_reset_ntf. It might be useful to have some doxygen
> comments with the declaration explaining what it does and when/why we
> should use it.
Looks like I hated the need for this one so much I forgot to document
it :S
I was hoping the core can issue all the callbacks just based on netdev
notifications, but it seems like most drivers have internal resets
procedures which require re-programming the ports :( Hence this helper
was born.
I'll document it like this:
* Called by the driver to inform the core that the entire UDP tunnel port
* state has been lost, usually due to device reset. Core will assume device
* forgot all the ports and issue .set_port and .sync_table callbacks as
* necessary.
Thanks for the review! I'll post v2 in a couple hours hoping someone
else will find time to review :S
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 4/9] ethtool: add tunnel info interface
2020-07-07 21:24 ` [PATCH net-next 4/9] ethtool: add tunnel info interface Jakub Kicinski
@ 2020-07-08 22:32 ` Michal Kubecek
2020-07-08 23:30 ` Jakub Kicinski
0 siblings, 1 reply; 17+ messages in thread
From: Michal Kubecek @ 2020-07-08 22:32 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, netdev, saeedm, michael.chan, edwin.peer, emil.s.tantilov,
alexander.h.duyck, jeffrey.t.kirsher, tariqt
On Tue, Jul 07, 2020 at 02:24:29PM -0700, Jakub Kicinski wrote:
> Add an interface to report offloaded UDP ports via ethtool netlink.
>
> Now that core takes care of tracking which UDP tunnel ports the NICs
> are aware of we can quite easily export this information out to
> user space.
>
> The responsibility of writing the netlink dumps is split between
> ethtool code and udp_tunnel_nic.c - since udp_tunnel module may
> not always be loaded, yet we should always report the capabilities
> of the NIC.
>
> $ ethtool --show-tunnels eth0
> Tunnel information for eth0:
> UDP port table 0:
> Size: 4
> Types: vxlan
> No entries
> UDP port table 1:
> Size: 4
> Types: geneve, vxlan-gpe
> Entries (1):
> port 1230, vxlan-gpe
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
> Documentation/networking/ethtool-netlink.rst | 33 +++
> include/net/udp_tunnel.h | 21 ++
> include/uapi/linux/ethtool.h | 2 +
> include/uapi/linux/ethtool_netlink.h | 55 ++++
> net/ethtool/Makefile | 3 +-
> net/ethtool/common.c | 9 +
> net/ethtool/common.h | 1 +
> net/ethtool/netlink.c | 12 +
> net/ethtool/netlink.h | 4 +
> net/ethtool/strset.c | 5 +
> net/ethtool/tunnels.c | 261 +++++++++++++++++++
> net/ipv4/udp_tunnel_nic.c | 69 +++++
> 12 files changed, 474 insertions(+), 1 deletion(-)
> create mode 100644 net/ethtool/tunnels.c
>
> diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst
> index 396390f4936b..6a9265401d31 100644
> --- a/Documentation/networking/ethtool-netlink.rst
> +++ b/Documentation/networking/ethtool-netlink.rst
> @@ -1230,6 +1230,39 @@ used to report the amplitude of the reflection for a given pair.
> | | | ``ETHTOOL_A_CABLE_AMPLITUDE_mV`` | s16 | Reflection amplitude |
> +-+-+-----------------------------------------+--------+----------------------+
>
> +TUNNEL_INFO
> +===========
> +
> +Gets information about the tunnel state NIC is aware of.
> +
> +Request contents:
> +
> + ===================================== ====== ==========================
> + ``ETHTOOL_A_TUNNEL_INFO_HEADER`` nested request header
> + ===================================== ====== ==========================
> +
> +Kernel response contents:
> +
> + +---------------------------------------------+--------+---------------------+
> + | ``ETHTOOL_A_TUNNEL_INFO_HEADER`` | nested | reply header |
> + +---------------------------------------------+--------+---------------------+
> + | ``ETHTOOL_A_TUNNEL_INFO_UDP_PORTS`` | nested | all UDP port tables |
> + +-+-------------------------------------------+--------+---------------------+
> + | | ``ETHTOOL_A_TUNNEL_UDP_TABLE`` | nested | one UDP port table |
> + +-+-+-----------------------------------------+--------+---------------------+
> + | | | ``ETHTOOL_A_TUNNEL_UDP_TABLE_SIZE`` | u32 | max size of the |
> + | | | | | table |
> + +-+-+-----------------------------------------+--------+---------------------+
> + | | | ``ETHTOOL_A_TUNNEL_UDP_TABLE_TYPES`` | u32 | bitmask of tunnel |
> + | | | | | types table can hold|
In the code below, this is a bitset, not u32.
> + +-+-+-----------------------------------------+--------+---------------------+
> + | | | ``ETHTOOL_A_TUNNEL_UDP_TABLE_ENTRY`` | nested | offloaded UDP port |
> + +-+-+-+---------------------------------------+--------+---------------------+
> + | | | | ``ETHTOOL_A_TUNNEL_UDP_ENTRY_PORT`` | be16 | UDP port |
> + +-+-+-+---------------------------------------+--------+---------------------+
> + | | | | ``ETHTOOL_A_TUNNEL_UDP_ENTRY_TYPE`` | u32 | tunnel type |
> + +-+-+-+---------------------------------------+--------+---------------------+
> +
> Request translation
> ===================
>
[...]
> diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
> index d1413538ef30..0495314ce20b 100644
> --- a/include/uapi/linux/ethtool.h
> +++ b/include/uapi/linux/ethtool.h
[...]
> @@ -556,6 +557,60 @@ enum {
> ETHTOOL_A_CABLE_TEST_TDR_NTF_MAX = __ETHTOOL_A_CABLE_TEST_TDR_NTF_CNT - 1
> };
>
> +/* TUNNEL INFO */
> +
> +enum {
> + ETHTOOL_A_TUNNEL_INFO_UNSPEC,
> + ETHTOOL_A_TUNNEL_INFO_HEADER, /* nest - _A_HEADER_* */
> +
> + ETHTOOL_A_TUNNEL_INFO_UDP_PORTS, /* nest - _UDP_TABLE */
> +
> + /* add new constants above here */
> + __ETHTOOL_A_TUNNEL_INFO_CNT,
> + ETHTOOL_A_TUNNEL_INFO_MAX = (__ETHTOOL_A_TUNNEL_INFO_CNT - 1)
> +};
nit: other requests with nested attributes have the enums ordered "from
inside out", i.e. nested attributes before the nest containing them.
> +
> +enum {
> + ETHTOOL_A_TUNNEL_UDP_UNSPEC,
> +
> + ETHTOOL_A_TUNNEL_UDP_TABLE, /* nest - _UDP_TABLE_* */
> +
> + /* add new constants above here */
> + __ETHTOOL_A_TUNNEL_UDP_CNT,
> + ETHTOOL_A_TUNNEL_UDP_MAX = (__ETHTOOL_A_TUNNEL_UDP_CNT - 1)
> +};
> +
> +enum {
> + ETHTOOL_A_TUNNEL_UDP_TABLE_UNSPEC,
> +
> + ETHTOOL_A_TUNNEL_UDP_TABLE_SIZE, /* u32 */
> + ETHTOOL_A_TUNNEL_UDP_TABLE_TYPES, /* u32 */
In the code below, this is a bitset, not u32.
> + ETHTOOL_A_TUNNEL_UDP_TABLE_ENTRY, /* nest - _UDP_ENTRY_* */
> +
> + /* add new constants above here */
> + __ETHTOOL_A_TUNNEL_UDP_TABLE_CNT,
> + ETHTOOL_A_TUNNEL_UDP_TABLE_MAX = (__ETHTOOL_A_TUNNEL_UDP_TABLE_CNT - 1)
> +};
> +
> +enum {
> + ETHTOOL_A_TUNNEL_UDP_ENTRY_UNSPEC,
> +
> + ETHTOOL_A_TUNNEL_UDP_ENTRY_PORT, /* be16 */
Do we get some benefit from passing the port in network byte order? It
would be helpful if we expected userspace to copy it e.g. into struct
sockaddr_in but that doesn't seem to be the case.
> + ETHTOOL_A_TUNNEL_UDP_ENTRY_TYPE, /* u32 */
> +
> + /* add new constants above here */
> + __ETHTOOL_A_TUNNEL_UDP_ENTRY_CNT,
> + ETHTOOL_A_TUNNEL_UDP_ENTRY_MAX = (__ETHTOOL_A_TUNNEL_UDP_ENTRY_CNT - 1)
> +};
> +
> +enum {
> + ETHTOOL_UDP_TUNNEL_TYPE_VXLAN,
> + ETHTOOL_UDP_TUNNEL_TYPE_GENEVE,
> + ETHTOOL_UDP_TUNNEL_TYPE_VXLAN_GPE,
> +
> + __ETHTOOL_UDP_TUNNEL_TYPE_BIT_CNT
nit: the "BIT" part looks inconsistent (like a leftover form an older
version where the constants were named differently).
> +};
> +
> /* generic netlink info */
> #define ETHTOOL_GENL_NAME "ethtool"
> #define ETHTOOL_GENL_VERSION 1
[...]
> diff --git a/net/ethtool/strset.c b/net/ethtool/strset.c
> index 0eed4e4909ab..5d3f315d4781 100644
> --- a/net/ethtool/strset.c
> +++ b/net/ethtool/strset.c
> @@ -75,6 +75,11 @@ static const struct strset_info info_template[] = {
> .count = __HWTSTAMP_FILTER_CNT,
> .strings = ts_rx_filter_names,
> },
> + [ETH_SS_UDP_TUNNEL_TYPES] = {
> + .per_dev = false,
> + .count = __ETHTOOL_A_TUNNEL_UDP_ENTRY_CNT,
This should be __ETHTOOL_UDP_TUNNEL_TYPE_BIT_CNT (number of strings in
the set, i.e. number of tunnel types).
> + .strings = udp_tunnel_type_names,
> + },
> };
>
> struct strset_req_info {
> diff --git a/net/ethtool/tunnels.c b/net/ethtool/tunnels.c
> new file mode 100644
> index 000000000000..e9e09ea08c6a
> --- /dev/null
> +++ b/net/ethtool/tunnels.c
> @@ -0,0 +1,261 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <linux/ethtool_netlink.h>
> +#include <net/udp_tunnel.h>
> +
> +#include "bitset.h"
> +#include "common.h"
> +#include "netlink.h"
> +
> +static const struct nla_policy
> +ethtool_tunnel_info_policy[ETHTOOL_A_TUNNEL_INFO_MAX + 1] = {
> + [ETHTOOL_A_TUNNEL_INFO_UNSPEC] = { .type = NLA_REJECT },
> + [ETHTOOL_A_TUNNEL_INFO_HEADER] = { .type = NLA_NESTED },
> +};
> +
> +static_assert(ETHTOOL_UDP_TUNNEL_TYPE_VXLAN == ilog2(UDP_TUNNEL_TYPE_VXLAN));
> +static_assert(ETHTOOL_UDP_TUNNEL_TYPE_GENEVE == ilog2(UDP_TUNNEL_TYPE_GENEVE));
> +static_assert(ETHTOOL_UDP_TUNNEL_TYPE_VXLAN_GPE ==
> + ilog2(UDP_TUNNEL_TYPE_VXLAN_GPE));
> +
> +static ssize_t
> +ethnl_tunnel_info_reply_size(const struct ethnl_req_info *req_base,
> + struct netlink_ext_ack *extack)
> +{
> + bool compact = req_base->flags & ETHTOOL_FLAG_COMPACT_BITSETS;
> + const struct udp_tunnel_nic_info *info;
> + unsigned int i;
> + size_t size;
> + int ret;
> +
> + BUILD_BUG_ON(__ETHTOOL_UDP_TUNNEL_TYPE_BIT_CNT > 32);
> +
> + info = req_base->dev->udp_tunnel_nic_info;
> + if (!info) {
> + NL_SET_ERR_MSG(extack,
> + "device does not report tunnel offload info");
> + return -EOPNOTSUPP;
> + }
> +
> + size = nla_total_size(0); /* _INFO_UDP_PORTS */
> +
> + for (i = 0; i < UDP_TUNNEL_NIC_MAX_TABLES; i++) {
> + if (!info->tables[i].n_entries)
> + return size;
> +
> + size += nla_total_size(0); /* _UDP_TABLE */
> + size += nla_total_size(sizeof(u32)); /* _UDP_TABLE_SIZE */
> + ret = ethnl_bitset32_size(&info->tables[i].tunnel_types, NULL,
> + __ETHTOOL_UDP_TUNNEL_TYPE_BIT_CNT,
> + udp_tunnel_type_names, compact);
> + if (ret < 0)
> + return ret;
> + size += ret;
> +
> + size += udp_tunnel_nic_dump_size(req_base->dev, i);
> + }
> +
> + return size;
> +}
How big can the message get? Can we be sure the information for one
device will always fit into a reasonably sized message? Attribute
ETHTOOL_A_TUNNEL_INFO_UDP_PORTS is limited by 65535 bytes (attribute
size is u16), can we always fit into this size?
Michal
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 7/9] ixgbe: convert to new udp_tunnel_nic infra
2020-07-08 21:25 ` Jakub Kicinski
@ 2020-07-08 23:14 ` Alexander Duyck
0 siblings, 0 replies; 17+ messages in thread
From: Alexander Duyck @ 2020-07-08 23:14 UTC (permalink / raw)
To: Jakub Kicinski
Cc: David Miller, Netdev, Saeed Mahameed, Michael Chan, edwin.peer,
Tantilov, Emil S, Alexander Duyck, Jeff Kirsher, Tariq Toukan,
Michal Kubecek
On Wed, Jul 8, 2020 at 2:25 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Wed, 8 Jul 2020 10:00:36 -0700 Alexander Duyck wrote:
> > On Tue, Jul 7, 2020 at 2:28 PM Jakub Kicinski <kuba@kernel.org> wrote:
> > >
> > > Make use of new common udp_tunnel_nic infra. ixgbe supports
> > > IPv4 only, and only single VxLAN and Geneve ports (one each).
> > >
> > > I'm dropping the confusing piece of code in ixgbe_set_features().
> > > ndo_udp_tunnel_add and ndo_udp_tunnel_del did not check if RXCSUM
> > > is enabled, so this code was either unnecessary or buggy anyway.
> >
> > The code is unnecessary from what I can tell. I suspect the reason for
> > adding the code was because the port values are used when performing
> > the Rx checksum offload. However we never disable it in hardware, and
> > the software path in ixgbe_rx_checksum will simply disable the related
> > code anyway since we cannot set skb->encapsulation if RXCSUM is
> > disabled.
> >
> > With that said moving this to a separate patch might be preferable as
> > it would make the patch more readable.
>
> Ack on all points!
>
> > > -static void ixgbe_clear_udp_tunnel_port(struct ixgbe_adapter *adapter, u32 mask)
> > > +static int ixgbe_udp_tunnel_sync(struct net_device *dev, unsigned int table)
> > > {
> > > + struct ixgbe_adapter *adapter = netdev_priv(dev);
> > > struct ixgbe_hw *hw = &adapter->hw;
> > > - u32 vxlanctrl;
> > > -
> > > - if (!(adapter->flags & (IXGBE_FLAG_VXLAN_OFFLOAD_CAPABLE |
> > > - IXGBE_FLAG_GENEVE_OFFLOAD_CAPABLE)))
> > > - return;
> > > + struct udp_tunnel_info ti;
> > >
> > > - vxlanctrl = IXGBE_READ_REG(hw, IXGBE_VXLANCTRL) & ~mask;
> > > - IXGBE_WRITE_REG(hw, IXGBE_VXLANCTRL, vxlanctrl);
> > > -
> > > - if (mask & IXGBE_VXLANCTRL_VXLAN_UDPPORT_MASK)
> > > - adapter->vxlan_port = 0;
> > > + udp_tunnel_nic_get_port(dev, table, 0, &ti);
> > > + if (!table)
> > > + adapter->vxlan_port = ti.port;
> > > + else
> > > + adapter->geneve_port = ti.port;
> >
> > So this !table thing is a bit hard to read. It might be more useful if
> > you had a define that made it clear that the expectation is that entry
> > 0 is a VXLAN table and entry 1 is the GENEVE.
>
> How about:
>
> if (ti.type == UDP_TUNNEL_TYPE_VXLAN)
>
> Tunnel info will have the type.
That would work too.
> > >
> > > - if (mask & IXGBE_VXLANCTRL_GENEVE_UDPPORT_MASK)
> > > - adapter->geneve_port = 0;
> > > + IXGBE_WRITE_REG(hw, IXGBE_VXLANCTRL,
> > > + ntohs(adapter->vxlan_port) |
> > > + ntohs(adapter->geneve_port) <<
> > > + IXGBE_VXLANCTRL_GENEVE_UDPPORT_SHIFT);
> > > + return 0;
> > > }
> >
> > I'm assuming the new logic will call this for all entries in the
> > tables regardless of if they are set or not. If so I suppose this is
> > fine.
>
> Yup, I think this is preferred form of sync for devices which just
> write registers. I wrote it for NFP initially but it seems to work
> in more places.
>
> The driver can query the entire table and will get either the port
> or 0 if port is not set.
Okay, sounds good.
> > > #ifdef CONFIG_IXGBE_DCB
> > > /**
> > > * ixgbe_configure_dcb - Configure DCB hardware
> > > @@ -6227,6 +6242,7 @@ static void ixgbe_init_dcb(struct ixgbe_adapter *adapter)
> > > /**
> > > * ixgbe_sw_init - Initialize general software structures (struct ixgbe_adapter)
> > > * @adapter: board private structure to initialize
> > > + * @netdev: network interface device structure
> > > * @ii: pointer to ixgbe_info for device
> > > *
> > > * ixgbe_sw_init initializes the Adapter private data structure.
> > > @@ -6234,6 +6250,7 @@ static void ixgbe_init_dcb(struct ixgbe_adapter *adapter)
> > > * OS network device settings (MTU size).
> > > **/
> > > static int ixgbe_sw_init(struct ixgbe_adapter *adapter,
> > > + struct net_device *netdev,
> > > const struct ixgbe_info *ii)
> > > {
> > > struct ixgbe_hw *hw = &adapter->hw;
> >
> > There is no need to add the argument. It should be accessible via
> > adapter->netdev, or for that matter you could pass the netdev and drop
> > the adapter and just pull it out via netdev_priv since the two are all
> > contained in the same structure anyway. Another option would be to
> > just pull the logic out of this section and put it in a switch
> > statement of its own in the probe function.
>
> Hm, new switch section definitely looks best. I'll do that.
>
> > > @@ -6332,7 +6349,7 @@ static int ixgbe_sw_init(struct ixgbe_adapter *adapter,
> > > adapter->flags2 |= IXGBE_FLAG2_TEMP_SENSOR_CAPABLE;
> > > break;
> > > case ixgbe_mac_x550em_a:
> > > - adapter->flags |= IXGBE_FLAG_GENEVE_OFFLOAD_CAPABLE;
> > > + netdev->udp_tunnel_nic_info = &ixgbe_udp_tunnels_x550em_a;
> > > switch (hw->device_id) {
> > > case IXGBE_DEV_ID_X550EM_A_1G_T:
> > > case IXGBE_DEV_ID_X550EM_A_1G_T_L:
> > > @@ -6359,7 +6376,8 @@ static int ixgbe_sw_init(struct ixgbe_adapter *adapter,
> > > #ifdef CONFIG_IXGBE_DCA
> > > adapter->flags &= ~IXGBE_FLAG_DCA_CAPABLE;
> > > #endif
> > > - adapter->flags |= IXGBE_FLAG_VXLAN_OFFLOAD_CAPABLE;
> > > + if (!netdev->udp_tunnel_nic_info)
> > > + netdev->udp_tunnel_nic_info = &ixgbe_udp_tunnels_x550;
> > > break;
> > > default:
> > > break;
> >
> > Not a fan of the !udp_tunnel_nic_info check, but I understand you are
> > having to do it because of the fall-through.
>
> Yup.
>
> > > @@ -6798,8 +6816,7 @@ int ixgbe_open(struct net_device *netdev)
> > >
> > > ixgbe_up_complete(adapter);
> > >
> > > - ixgbe_clear_udp_tunnel_port(adapter, IXGBE_VXLANCTRL_ALL_UDPPORT_MASK);
> > > - udp_tunnel_get_rx_info(netdev);
> > > + udp_tunnel_nic_reset_ntf(netdev);
> > >
> > > return 0;
> >
> > So I went looking through the earlier patches for an explanation of
> > udp_tunnel_nic_reset_ntf. It might be useful to have some doxygen
> > comments with the declaration explaining what it does and when/why we
> > should use it.
>
> Looks like I hated the need for this one so much I forgot to document
> it :S
>
> I was hoping the core can issue all the callbacks just based on netdev
> notifications, but it seems like most drivers have internal resets
> procedures which require re-programming the ports :( Hence this helper
> was born.
>
> I'll document it like this:
>
> * Called by the driver to inform the core that the entire UDP tunnel port
> * state has been lost, usually due to device reset. Core will assume device
> * forgot all the ports and issue .set_port and .sync_table callbacks as
> * necessary.
>
>
> Thanks for the review! I'll post v2 in a couple hours hoping someone
> else will find time to review :S
Thanks, this does appear to do quite a bit to simplify the driver code.
- Alex
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 4/9] ethtool: add tunnel info interface
2020-07-08 22:32 ` Michal Kubecek
@ 2020-07-08 23:30 ` Jakub Kicinski
2020-07-09 0:03 ` Michal Kubecek
0 siblings, 1 reply; 17+ messages in thread
From: Jakub Kicinski @ 2020-07-08 23:30 UTC (permalink / raw)
To: Michal Kubecek
Cc: davem, netdev, saeedm, michael.chan, edwin.peer, emil.s.tantilov,
alexander.h.duyck, jeffrey.t.kirsher, tariqt
On Thu, 9 Jul 2020 00:32:24 +0200 Michal Kubecek wrote:
> On Tue, Jul 07, 2020 at 02:24:29PM -0700, Jakub Kicinski wrote:
> > + +---------------------------------------------+--------+---------------------+
> > + | ``ETHTOOL_A_TUNNEL_INFO_HEADER`` | nested | reply header |
> > + +---------------------------------------------+--------+---------------------+
> > + | ``ETHTOOL_A_TUNNEL_INFO_UDP_PORTS`` | nested | all UDP port tables |
> > + +-+-------------------------------------------+--------+---------------------+
> > + | | ``ETHTOOL_A_TUNNEL_UDP_TABLE`` | nested | one UDP port table |
> > + +-+-+-----------------------------------------+--------+---------------------+
> > + | | | ``ETHTOOL_A_TUNNEL_UDP_TABLE_SIZE`` | u32 | max size of the |
> > + | | | | | table |
> > + +-+-+-----------------------------------------+--------+---------------------+
> > + | | | ``ETHTOOL_A_TUNNEL_UDP_TABLE_TYPES`` | u32 | bitmask of tunnel |
> > + | | | | | types table can hold|
>
> In the code below, this is a bitset, not u32.
I was going back and forth on the type. I'll update the doc to say
bitset, LMK if you prefer u32.
> > + +-+-+-----------------------------------------+--------+---------------------+
> > + | | | ``ETHTOOL_A_TUNNEL_UDP_TABLE_ENTRY`` | nested | offloaded UDP port |
> > + +-+-+-+---------------------------------------+--------+---------------------+
> > + | | | | ``ETHTOOL_A_TUNNEL_UDP_ENTRY_PORT`` | be16 | UDP port |
> > + +-+-+-+---------------------------------------+--------+---------------------+
> > + | | | | ``ETHTOOL_A_TUNNEL_UDP_ENTRY_TYPE`` | u32 | tunnel type |
> > + +-+-+-+---------------------------------------+--------+---------------------+
> > +enum {
> > + ETHTOOL_A_TUNNEL_INFO_UNSPEC,
> > + ETHTOOL_A_TUNNEL_INFO_HEADER, /* nest - _A_HEADER_* */
> > +
> > + ETHTOOL_A_TUNNEL_INFO_UDP_PORTS, /* nest - _UDP_TABLE */
> > +
> > + /* add new constants above here */
> > + __ETHTOOL_A_TUNNEL_INFO_CNT,
> > + ETHTOOL_A_TUNNEL_INFO_MAX = (__ETHTOOL_A_TUNNEL_INFO_CNT - 1)
> > +};
>
> nit: other requests with nested attributes have the enums ordered "from
> inside out", i.e. nested attributes before the nest containing them.
I see! I'll reorder.
> > + ETHTOOL_A_TUNNEL_UDP_TABLE_ENTRY, /* nest - _UDP_ENTRY_* */
> > +
> > + /* add new constants above here */
> > + __ETHTOOL_A_TUNNEL_UDP_TABLE_CNT,
> > + ETHTOOL_A_TUNNEL_UDP_TABLE_MAX = (__ETHTOOL_A_TUNNEL_UDP_TABLE_CNT - 1)
> > +};
> > +
> > +enum {
> > + ETHTOOL_A_TUNNEL_UDP_ENTRY_UNSPEC,
> > +
> > + ETHTOOL_A_TUNNEL_UDP_ENTRY_PORT, /* be16 */
>
> Do we get some benefit from passing the port in network byte order? It
> would be helpful if we expected userspace to copy it e.g. into struct
> sockaddr_in but that doesn't seem to be the case.
I was just following what I believe is a more common pattern.
TC uses be16 AFAIK (flower, act_ct, tunnel), so do the tunnels
(IFLA_GENEVE_PORT, IFLA_VXLAN_PORT). And ethtool flow descriptions.
I can change, but IMHO that'd be more surprising than be16.
> > + ETHTOOL_A_TUNNEL_UDP_ENTRY_TYPE, /* u32 */
> > +
> > + /* add new constants above here */
> > + __ETHTOOL_A_TUNNEL_UDP_ENTRY_CNT,
> > + ETHTOOL_A_TUNNEL_UDP_ENTRY_MAX = (__ETHTOOL_A_TUNNEL_UDP_ENTRY_CNT - 1)
> > +};
> > +
> > +enum {
> > + ETHTOOL_UDP_TUNNEL_TYPE_VXLAN,
> > + ETHTOOL_UDP_TUNNEL_TYPE_GENEVE,
> > + ETHTOOL_UDP_TUNNEL_TYPE_VXLAN_GPE,
> > +
> > + __ETHTOOL_UDP_TUNNEL_TYPE_BIT_CNT
>
> nit: the "BIT" part looks inconsistent (like a leftover form an older
> version where the constants were named differently).
Ah, thanks, missed in rename.
> > +};
> > +
> > /* generic netlink info */
> > #define ETHTOOL_GENL_NAME "ethtool"
> > #define ETHTOOL_GENL_VERSION 1
> [...]
> > diff --git a/net/ethtool/strset.c b/net/ethtool/strset.c
> > index 0eed4e4909ab..5d3f315d4781 100644
> > --- a/net/ethtool/strset.c
> > +++ b/net/ethtool/strset.c
> > @@ -75,6 +75,11 @@ static const struct strset_info info_template[] = {
> > .count = __HWTSTAMP_FILTER_CNT,
> > .strings = ts_rx_filter_names,
> > },
> > + [ETH_SS_UDP_TUNNEL_TYPES] = {
> > + .per_dev = false,
> > + .count = __ETHTOOL_A_TUNNEL_UDP_ENTRY_CNT,
>
> This should be __ETHTOOL_UDP_TUNNEL_TYPE_BIT_CNT (number of strings in
> the set, i.e. number of tunnel types).
:o Ack
> > + .strings = udp_tunnel_type_names,
> > + },
> > };
> >
> > struct strset_req_info {
> > diff --git a/net/ethtool/tunnels.c b/net/ethtool/tunnels.c
> > new file mode 100644
> > index 000000000000..e9e09ea08c6a
> > --- /dev/null
> > +++ b/net/ethtool/tunnels.c
> > @@ -0,0 +1,261 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +
> > +#include <linux/ethtool_netlink.h>
> > +#include <net/udp_tunnel.h>
> > +
> > +#include "bitset.h"
> > +#include "common.h"
> > +#include "netlink.h"
> > +
> > +static const struct nla_policy
> > +ethtool_tunnel_info_policy[ETHTOOL_A_TUNNEL_INFO_MAX + 1] = {
> > + [ETHTOOL_A_TUNNEL_INFO_UNSPEC] = { .type = NLA_REJECT },
> > + [ETHTOOL_A_TUNNEL_INFO_HEADER] = { .type = NLA_NESTED },
> > +};
> > +
> > +static_assert(ETHTOOL_UDP_TUNNEL_TYPE_VXLAN == ilog2(UDP_TUNNEL_TYPE_VXLAN));
> > +static_assert(ETHTOOL_UDP_TUNNEL_TYPE_GENEVE == ilog2(UDP_TUNNEL_TYPE_GENEVE));
> > +static_assert(ETHTOOL_UDP_TUNNEL_TYPE_VXLAN_GPE ==
> > + ilog2(UDP_TUNNEL_TYPE_VXLAN_GPE));
> > +
> > +static ssize_t
> > +ethnl_tunnel_info_reply_size(const struct ethnl_req_info *req_base,
> > + struct netlink_ext_ack *extack)
> > +{
> > + bool compact = req_base->flags & ETHTOOL_FLAG_COMPACT_BITSETS;
> > + const struct udp_tunnel_nic_info *info;
> > + unsigned int i;
> > + size_t size;
> > + int ret;
> > +
> > + BUILD_BUG_ON(__ETHTOOL_UDP_TUNNEL_TYPE_BIT_CNT > 32);
> > +
> > + info = req_base->dev->udp_tunnel_nic_info;
> > + if (!info) {
> > + NL_SET_ERR_MSG(extack,
> > + "device does not report tunnel offload info");
> > + return -EOPNOTSUPP;
> > + }
> > +
> > + size = nla_total_size(0); /* _INFO_UDP_PORTS */
> > +
> > + for (i = 0; i < UDP_TUNNEL_NIC_MAX_TABLES; i++) {
> > + if (!info->tables[i].n_entries)
> > + return size;
> > +
> > + size += nla_total_size(0); /* _UDP_TABLE */
> > + size += nla_total_size(sizeof(u32)); /* _UDP_TABLE_SIZE */
> > + ret = ethnl_bitset32_size(&info->tables[i].tunnel_types, NULL,
> > + __ETHTOOL_UDP_TUNNEL_TYPE_BIT_CNT,
> > + udp_tunnel_type_names, compact);
> > + if (ret < 0)
> > + return ret;
> > + size += ret;
> > +
> > + size += udp_tunnel_nic_dump_size(req_base->dev, i);
> > + }
> > +
> > + return size;
> > +}
>
> How big can the message get? Can we be sure the information for one
> device will always fit into a reasonably sized message? Attribute
> ETHTOOL_A_TUNNEL_INFO_UDP_PORTS is limited by 65535 bytes (attribute
> size is u16), can we always fit into this size?
I don't think I've seen any driver with more than 2 tables
or 16 entries total, and they don't seem to be growing in newer
HW (people tend to use standard ports).
188B + 16 * 20B = 508B - so we should be pretty safe with 64k.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 4/9] ethtool: add tunnel info interface
2020-07-08 23:30 ` Jakub Kicinski
@ 2020-07-09 0:03 ` Michal Kubecek
0 siblings, 0 replies; 17+ messages in thread
From: Michal Kubecek @ 2020-07-09 0:03 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, netdev, saeedm, michael.chan, edwin.peer, emil.s.tantilov,
alexander.h.duyck, jeffrey.t.kirsher, tariqt
On Wed, Jul 08, 2020 at 04:30:49PM -0700, Jakub Kicinski wrote:
> > > + ETHTOOL_A_TUNNEL_UDP_TABLE_ENTRY, /* nest - _UDP_ENTRY_* */
> > > +
> > > + /* add new constants above here */
> > > + __ETHTOOL_A_TUNNEL_UDP_TABLE_CNT,
> > > + ETHTOOL_A_TUNNEL_UDP_TABLE_MAX = (__ETHTOOL_A_TUNNEL_UDP_TABLE_CNT - 1)
> > > +};
> > > +
> > > +enum {
> > > + ETHTOOL_A_TUNNEL_UDP_ENTRY_UNSPEC,
> > > +
> > > + ETHTOOL_A_TUNNEL_UDP_ENTRY_PORT, /* be16 */
> >
> > Do we get some benefit from passing the port in network byte order? It
> > would be helpful if we expected userspace to copy it e.g. into struct
> > sockaddr_in but that doesn't seem to be the case.
Let's leave it as be16 for consistency then.
> > How big can the message get? Can we be sure the information for one
> > device will always fit into a reasonably sized message? Attribute
> > ETHTOOL_A_TUNNEL_INFO_UDP_PORTS is limited by 65535 bytes (attribute
> > size is u16), can we always fit into this size?
>
> I don't think I've seen any driver with more than 2 tables
> or 16 entries total, and they don't seem to be growing in newer
> HW (people tend to use standard ports).
>
> 188B + 16 * 20B = 508B - so we should be pretty safe with 64k.
So we are safe even if things grow by a factor of 100. Sounds good
enough, thanks.
Michal
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2020-07-09 0:03 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-07 21:24 [PATCH net-next 0/9] udp_tunnel: add NIC RX port offload infrastructure Jakub Kicinski
2020-07-07 21:24 ` [PATCH net-next 1/9] debugfs: make sure we can remove u32_array files cleanly Jakub Kicinski
2020-07-08 7:22 ` Greg Kroah-Hartman
2020-07-07 21:24 ` [PATCH net-next 2/9] udp_tunnel: re-number the offload tunnel types Jakub Kicinski
2020-07-07 21:24 ` [PATCH net-next 3/9] udp_tunnel: add central NIC RX port offload infrastructure Jakub Kicinski
2020-07-07 21:24 ` [PATCH net-next 4/9] ethtool: add tunnel info interface Jakub Kicinski
2020-07-08 22:32 ` Michal Kubecek
2020-07-08 23:30 ` Jakub Kicinski
2020-07-09 0:03 ` Michal Kubecek
2020-07-07 21:24 ` [PATCH net-next 5/9] netdevsim: add UDP tunnel port offload support Jakub Kicinski
2020-07-07 21:24 ` [PATCH net-next 6/9] selftests: net: add a test for UDP tunnel info infra Jakub Kicinski
2020-07-07 21:24 ` [PATCH net-next 7/9] ixgbe: convert to new udp_tunnel_nic infra Jakub Kicinski
2020-07-08 17:00 ` Alexander Duyck
2020-07-08 21:25 ` Jakub Kicinski
2020-07-08 23:14 ` Alexander Duyck
2020-07-07 21:24 ` [PATCH net-next 8/9] bnxt: " Jakub Kicinski
2020-07-07 21:24 ` [PATCH net-next 9/9] mlx4: " Jakub Kicinski
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).