v2 ethtool: remove support for ETHTOOL_GRXNTUPLE
This change is meant to remove all support for displaying an ntuple as
strings via ETHTOOL_GRXNTUPLE. The reason for this change is due to the
fact that multiple issues have been found including:
- Multiple buffer overruns for strings being displayed.
- Incorrect filters displayed, cleared filters with ring of -2 are displayed
- Setting get_rx_ntuple displays no rules if defined.
- Endianess wrong on displayed values.
- Hard limit of 1024 filters makes display functionality extremely limited
The only driver that had supported this interface was ixgbe. Since it no
longer uses the interface and due to the issues mentioned above I am
submitting this patch to remove it.
v2:
Updated based on comments from Ben Hutchings
- Left ETH_SS_NTUPLE_FILTERS in code but commented on it being deprecated
- Removed ethtool_rx_ntuple_list and ethtool_rx_ntuple_flow_spec_container
- Left ETHTOOL_GRXNTUPLE but commented it as deprecated
Also cleaned up set_rx_ntuple since there is no flow spec container to
maintain we can drop all the code for the alloc and free of it and just
return ops->set_rx_ntuple().
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Acked-by: Ben Hutchings <bhutchings@solarflare.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index fd14116..b7c12a6 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -169,18 +169,6 @@
}
EXPORT_SYMBOL(ethtool_op_set_flags);
-void ethtool_ntuple_flush(struct net_device *dev)
-{
- struct ethtool_rx_ntuple_flow_spec_container *fsc, *f;
-
- list_for_each_entry_safe(fsc, f, &dev->ethtool_ntuple_list.list, list) {
- list_del(&fsc->list);
- kfree(fsc);
- }
- dev->ethtool_ntuple_list.count = 0;
-}
-EXPORT_SYMBOL(ethtool_ntuple_flush);
-
/* Handlers for each ethtool command */
#define ETHTOOL_DEV_FEATURE_WORDS 1
@@ -865,34 +853,6 @@
return ret;
}
-static void __rx_ntuple_filter_add(struct ethtool_rx_ntuple_list *list,
- struct ethtool_rx_ntuple_flow_spec *spec,
- struct ethtool_rx_ntuple_flow_spec_container *fsc)
-{
-
- /* don't add filters forever */
- if (list->count >= ETHTOOL_MAX_NTUPLE_LIST_ENTRY) {
- /* free the container */
- kfree(fsc);
- return;
- }
-
- /* Copy the whole filter over */
- fsc->fs.flow_type = spec->flow_type;
- memcpy(&fsc->fs.h_u, &spec->h_u, sizeof(spec->h_u));
- memcpy(&fsc->fs.m_u, &spec->m_u, sizeof(spec->m_u));
-
- fsc->fs.vlan_tag = spec->vlan_tag;
- fsc->fs.vlan_tag_mask = spec->vlan_tag_mask;
- fsc->fs.data = spec->data;
- fsc->fs.data_mask = spec->data_mask;
- fsc->fs.action = spec->action;
-
- /* add to the list */
- list_add_tail_rcu(&fsc->list, &list->list);
- list->count++;
-}
-
/*
* ethtool does not (or did not) set masks for flow parameters that are
* not specified, so if both value and mask are 0 then this must be
@@ -930,8 +890,6 @@
{
struct ethtool_rx_ntuple cmd;
const struct ethtool_ops *ops = dev->ethtool_ops;
- struct ethtool_rx_ntuple_flow_spec_container *fsc = NULL;
- int ret;
if (!ops->set_rx_ntuple)
return -EOPNOTSUPP;
@@ -944,269 +902,7 @@
rx_ntuple_fix_masks(&cmd.fs);
- /*
- * Cache filter in dev struct for GET operation only if
- * the underlying driver doesn't have its own GET operation, and
- * only if the filter was added successfully. First make sure we
- * can allocate the filter, then continue if successful.
- */
- if (!ops->get_rx_ntuple) {
- fsc = kmalloc(sizeof(*fsc), GFP_ATOMIC);
- if (!fsc)
- return -ENOMEM;
- }
-
- ret = ops->set_rx_ntuple(dev, &cmd);
- if (ret) {
- kfree(fsc);
- return ret;
- }
-
- if (!ops->get_rx_ntuple)
- __rx_ntuple_filter_add(&dev->ethtool_ntuple_list, &cmd.fs, fsc);
-
- return ret;
-}
-
-static int ethtool_get_rx_ntuple(struct net_device *dev, void __user *useraddr)
-{
- struct ethtool_gstrings gstrings;
- const struct ethtool_ops *ops = dev->ethtool_ops;
- struct ethtool_rx_ntuple_flow_spec_container *fsc;
- u8 *data;
- char *p;
- int ret, i, num_strings = 0;
-
- if (!ops->get_sset_count)
- return -EOPNOTSUPP;
-
- if (copy_from_user(&gstrings, useraddr, sizeof(gstrings)))
- return -EFAULT;
-
- ret = ops->get_sset_count(dev, gstrings.string_set);
- if (ret < 0)
- return ret;
-
- gstrings.len = ret;
-
- data = kzalloc(gstrings.len * ETH_GSTRING_LEN, GFP_USER);
- if (!data)
- return -ENOMEM;
-
- if (ops->get_rx_ntuple) {
- /* driver-specific filter grab */
- ret = ops->get_rx_ntuple(dev, gstrings.string_set, data);
- goto copy;
- }
-
- /* default ethtool filter grab */
- i = 0;
- p = (char *)data;
- list_for_each_entry(fsc, &dev->ethtool_ntuple_list.list, list) {
- sprintf(p, "Filter %d:\n", i);
- p += ETH_GSTRING_LEN;
- num_strings++;
-
- switch (fsc->fs.flow_type) {
- case TCP_V4_FLOW:
- sprintf(p, "\tFlow Type: TCP\n");
- p += ETH_GSTRING_LEN;
- num_strings++;
- break;
- case UDP_V4_FLOW:
- sprintf(p, "\tFlow Type: UDP\n");
- p += ETH_GSTRING_LEN;
- num_strings++;
- break;
- case SCTP_V4_FLOW:
- sprintf(p, "\tFlow Type: SCTP\n");
- p += ETH_GSTRING_LEN;
- num_strings++;
- break;
- case AH_ESP_V4_FLOW:
- sprintf(p, "\tFlow Type: AH ESP\n");
- p += ETH_GSTRING_LEN;
- num_strings++;
- break;
- case ESP_V4_FLOW:
- sprintf(p, "\tFlow Type: ESP\n");
- p += ETH_GSTRING_LEN;
- num_strings++;
- break;
- case IP_USER_FLOW:
- sprintf(p, "\tFlow Type: Raw IP\n");
- p += ETH_GSTRING_LEN;
- num_strings++;
- break;
- case IPV4_FLOW:
- sprintf(p, "\tFlow Type: IPv4\n");
- p += ETH_GSTRING_LEN;
- num_strings++;
- break;
- default:
- sprintf(p, "\tFlow Type: Unknown\n");
- p += ETH_GSTRING_LEN;
- num_strings++;
- goto unknown_filter;
- }
-
- /* now the rest of the filters */
- switch (fsc->fs.flow_type) {
- case TCP_V4_FLOW:
- case UDP_V4_FLOW:
- case SCTP_V4_FLOW:
- sprintf(p, "\tSrc IP addr: 0x%x\n",
- fsc->fs.h_u.tcp_ip4_spec.ip4src);
- p += ETH_GSTRING_LEN;
- num_strings++;
- sprintf(p, "\tSrc IP mask: 0x%x\n",
- fsc->fs.m_u.tcp_ip4_spec.ip4src);
- p += ETH_GSTRING_LEN;
- num_strings++;
- sprintf(p, "\tDest IP addr: 0x%x\n",
- fsc->fs.h_u.tcp_ip4_spec.ip4dst);
- p += ETH_GSTRING_LEN;
- num_strings++;
- sprintf(p, "\tDest IP mask: 0x%x\n",
- fsc->fs.m_u.tcp_ip4_spec.ip4dst);
- p += ETH_GSTRING_LEN;
- num_strings++;
- sprintf(p, "\tSrc Port: %d, mask: 0x%x\n",
- fsc->fs.h_u.tcp_ip4_spec.psrc,
- fsc->fs.m_u.tcp_ip4_spec.psrc);
- p += ETH_GSTRING_LEN;
- num_strings++;
- sprintf(p, "\tDest Port: %d, mask: 0x%x\n",
- fsc->fs.h_u.tcp_ip4_spec.pdst,
- fsc->fs.m_u.tcp_ip4_spec.pdst);
- p += ETH_GSTRING_LEN;
- num_strings++;
- sprintf(p, "\tTOS: %d, mask: 0x%x\n",
- fsc->fs.h_u.tcp_ip4_spec.tos,
- fsc->fs.m_u.tcp_ip4_spec.tos);
- p += ETH_GSTRING_LEN;
- num_strings++;
- break;
- case AH_ESP_V4_FLOW:
- case ESP_V4_FLOW:
- sprintf(p, "\tSrc IP addr: 0x%x\n",
- fsc->fs.h_u.ah_ip4_spec.ip4src);
- p += ETH_GSTRING_LEN;
- num_strings++;
- sprintf(p, "\tSrc IP mask: 0x%x\n",
- fsc->fs.m_u.ah_ip4_spec.ip4src);
- p += ETH_GSTRING_LEN;
- num_strings++;
- sprintf(p, "\tDest IP addr: 0x%x\n",
- fsc->fs.h_u.ah_ip4_spec.ip4dst);
- p += ETH_GSTRING_LEN;
- num_strings++;
- sprintf(p, "\tDest IP mask: 0x%x\n",
- fsc->fs.m_u.ah_ip4_spec.ip4dst);
- p += ETH_GSTRING_LEN;
- num_strings++;
- sprintf(p, "\tSPI: %d, mask: 0x%x\n",
- fsc->fs.h_u.ah_ip4_spec.spi,
- fsc->fs.m_u.ah_ip4_spec.spi);
- p += ETH_GSTRING_LEN;
- num_strings++;
- sprintf(p, "\tTOS: %d, mask: 0x%x\n",
- fsc->fs.h_u.ah_ip4_spec.tos,
- fsc->fs.m_u.ah_ip4_spec.tos);
- p += ETH_GSTRING_LEN;
- num_strings++;
- break;
- case IP_USER_FLOW:
- sprintf(p, "\tSrc IP addr: 0x%x\n",
- fsc->fs.h_u.usr_ip4_spec.ip4src);
- p += ETH_GSTRING_LEN;
- num_strings++;
- sprintf(p, "\tSrc IP mask: 0x%x\n",
- fsc->fs.m_u.usr_ip4_spec.ip4src);
- p += ETH_GSTRING_LEN;
- num_strings++;
- sprintf(p, "\tDest IP addr: 0x%x\n",
- fsc->fs.h_u.usr_ip4_spec.ip4dst);
- p += ETH_GSTRING_LEN;
- num_strings++;
- sprintf(p, "\tDest IP mask: 0x%x\n",
- fsc->fs.m_u.usr_ip4_spec.ip4dst);
- p += ETH_GSTRING_LEN;
- num_strings++;
- break;
- case IPV4_FLOW:
- sprintf(p, "\tSrc IP addr: 0x%x\n",
- fsc->fs.h_u.usr_ip4_spec.ip4src);
- p += ETH_GSTRING_LEN;
- num_strings++;
- sprintf(p, "\tSrc IP mask: 0x%x\n",
- fsc->fs.m_u.usr_ip4_spec.ip4src);
- p += ETH_GSTRING_LEN;
- num_strings++;
- sprintf(p, "\tDest IP addr: 0x%x\n",
- fsc->fs.h_u.usr_ip4_spec.ip4dst);
- p += ETH_GSTRING_LEN;
- num_strings++;
- sprintf(p, "\tDest IP mask: 0x%x\n",
- fsc->fs.m_u.usr_ip4_spec.ip4dst);
- p += ETH_GSTRING_LEN;
- num_strings++;
- sprintf(p, "\tL4 bytes: 0x%x, mask: 0x%x\n",
- fsc->fs.h_u.usr_ip4_spec.l4_4_bytes,
- fsc->fs.m_u.usr_ip4_spec.l4_4_bytes);
- p += ETH_GSTRING_LEN;
- num_strings++;
- sprintf(p, "\tTOS: %d, mask: 0x%x\n",
- fsc->fs.h_u.usr_ip4_spec.tos,
- fsc->fs.m_u.usr_ip4_spec.tos);
- p += ETH_GSTRING_LEN;
- num_strings++;
- sprintf(p, "\tIP Version: %d, mask: 0x%x\n",
- fsc->fs.h_u.usr_ip4_spec.ip_ver,
- fsc->fs.m_u.usr_ip4_spec.ip_ver);
- p += ETH_GSTRING_LEN;
- num_strings++;
- sprintf(p, "\tProtocol: %d, mask: 0x%x\n",
- fsc->fs.h_u.usr_ip4_spec.proto,
- fsc->fs.m_u.usr_ip4_spec.proto);
- p += ETH_GSTRING_LEN;
- num_strings++;
- break;
- }
- sprintf(p, "\tVLAN: %d, mask: 0x%x\n",
- fsc->fs.vlan_tag, fsc->fs.vlan_tag_mask);
- p += ETH_GSTRING_LEN;
- num_strings++;
- sprintf(p, "\tUser-defined: 0x%Lx\n", fsc->fs.data);
- p += ETH_GSTRING_LEN;
- num_strings++;
- sprintf(p, "\tUser-defined mask: 0x%Lx\n", fsc->fs.data_mask);
- p += ETH_GSTRING_LEN;
- num_strings++;
- if (fsc->fs.action == ETHTOOL_RXNTUPLE_ACTION_DROP)
- sprintf(p, "\tAction: Drop\n");
- else
- sprintf(p, "\tAction: Direct to queue %d\n",
- fsc->fs.action);
- p += ETH_GSTRING_LEN;
- num_strings++;
-unknown_filter:
- i++;
- }
-copy:
- /* indicate to userspace how many strings we actually have */
- gstrings.len = num_strings;
- ret = -EFAULT;
- if (copy_to_user(useraddr, &gstrings, sizeof(gstrings)))
- goto out;
- useraddr += sizeof(gstrings);
- if (copy_to_user(useraddr, data, gstrings.len * ETH_GSTRING_LEN))
- goto out;
- ret = 0;
-
-out:
- kfree(data);
- return ret;
+ return ops->set_rx_ntuple(dev, &cmd);
}
static int ethtool_get_regs(struct net_device *dev, char __user *useraddr)
@@ -2101,9 +1797,6 @@
case ETHTOOL_SRXNTUPLE:
rc = ethtool_set_rx_ntuple(dev, useraddr);
break;
- case ETHTOOL_GRXNTUPLE:
- rc = ethtool_get_rx_ntuple(dev, useraddr);
- break;
case ETHTOOL_GSSET_INFO:
rc = ethtool_get_sset_info(dev, useraddr);
break;