* [RFC PATCH net-next v3 14/21] ethtool: provide link mode names as a string set
From: Michal Kubecek @ 2019-02-18 18:22 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Andrew Lunn, Jakub Kicinski, Jiri Pirko,
linux-kernel
In-Reply-To: <cover.1550513384.git.mkubecek@suse.cz>
Add table of ethernet link mode names and make it available as a string set
to userspace GET_STRSET requests.
Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
---
include/uapi/linux/ethtool.h | 2 ++
net/ethtool/netlink.c | 58 ++++++++++++++++++++++++++++++++++++
net/ethtool/netlink.h | 1 +
net/ethtool/strset.c | 6 ++++
4 files changed, 67 insertions(+)
diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index 1b58637d3a4d..ba96a691bfd4 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -566,6 +566,7 @@ struct ethtool_pauseparam {
* @ETH_SS_TSTAMP_SOF: timestamping flag names
* @ETH_SS_TSTAMP_TX_TYPE: timestamping Tx type names
* @ETH_SS_TSTAMP_RX_FILTER: timestamping Rx filter names
+ * @ETH_SS_LINK_MODES: link mode names
*/
enum ethtool_stringset {
ETH_SS_TEST = 0,
@@ -580,6 +581,7 @@ enum ethtool_stringset {
ETH_SS_TSTAMP_SOF,
ETH_SS_TSTAMP_TX_TYPE,
ETH_SS_TSTAMP_RX_FILTER,
+ ETH_SS_LINK_MODES,
ETH_SS_COUNT
};
diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
index e27dec427414..1ff6696ad716 100644
--- a/net/ethtool/netlink.c
+++ b/net/ethtool/netlink.c
@@ -6,6 +6,61 @@
u32 ethnl_bcast_seq;
+const char *const link_mode_names[] = {
+ [ETHTOOL_LINK_MODE_10baseT_Half_BIT] = "10baseT/Half",
+ [ETHTOOL_LINK_MODE_10baseT_Full_BIT] = "10baseT/Full",
+ [ETHTOOL_LINK_MODE_100baseT_Half_BIT] = "100baseT/Half",
+ [ETHTOOL_LINK_MODE_100baseT_Full_BIT] = "100baseT/Full",
+ [ETHTOOL_LINK_MODE_1000baseT_Half_BIT] = "1000baseT/Half",
+ [ETHTOOL_LINK_MODE_1000baseT_Full_BIT] = "1000baseT/Full",
+ [ETHTOOL_LINK_MODE_Autoneg_BIT] = "Autoneg",
+ [ETHTOOL_LINK_MODE_TP_BIT] = "TP",
+ [ETHTOOL_LINK_MODE_AUI_BIT] = "AUI",
+ [ETHTOOL_LINK_MODE_MII_BIT] = "MII",
+ [ETHTOOL_LINK_MODE_FIBRE_BIT] = "FIBRE",
+ [ETHTOOL_LINK_MODE_BNC_BIT] = "BNC",
+ [ETHTOOL_LINK_MODE_10000baseT_Full_BIT] = "10000baseT/Full",
+ [ETHTOOL_LINK_MODE_Pause_BIT] = "Pause",
+ [ETHTOOL_LINK_MODE_Asym_Pause_BIT] = "Asym_Pause",
+ [ETHTOOL_LINK_MODE_2500baseX_Full_BIT] = "2500baseX/Full",
+ [ETHTOOL_LINK_MODE_Backplane_BIT] = "Backplane",
+ [ETHTOOL_LINK_MODE_1000baseKX_Full_BIT] = "1000baseKX/Full",
+ [ETHTOOL_LINK_MODE_10000baseKX4_Full_BIT] = "10000baseKX4/Full",
+ [ETHTOOL_LINK_MODE_10000baseKR_Full_BIT] = "10000baseKR/Full",
+ [ETHTOOL_LINK_MODE_10000baseR_FEC_BIT] = "10000baseR/FEC",
+ [ETHTOOL_LINK_MODE_20000baseMLD2_Full_BIT] = "20000baseMLD2/Full",
+ [ETHTOOL_LINK_MODE_20000baseKR2_Full_BIT] = "20000baseKR2/Full",
+ [ETHTOOL_LINK_MODE_40000baseKR4_Full_BIT] = "40000baseKR4/Full",
+ [ETHTOOL_LINK_MODE_40000baseCR4_Full_BIT] = "40000baseCR4/Full",
+ [ETHTOOL_LINK_MODE_40000baseSR4_Full_BIT] = "40000baseSR4/Full",
+ [ETHTOOL_LINK_MODE_40000baseLR4_Full_BIT] = "40000baseLR4/Full",
+ [ETHTOOL_LINK_MODE_56000baseKR4_Full_BIT] = "56000baseKR4/Full",
+ [ETHTOOL_LINK_MODE_56000baseCR4_Full_BIT] = "56000baseCR4/Full",
+ [ETHTOOL_LINK_MODE_56000baseSR4_Full_BIT] = "56000baseSR4/Full",
+ [ETHTOOL_LINK_MODE_56000baseLR4_Full_BIT] = "56000baseLR4/Full",
+ [ETHTOOL_LINK_MODE_25000baseCR_Full_BIT] = "25000baseCR/Full",
+ [ETHTOOL_LINK_MODE_25000baseKR_Full_BIT] = "25000baseKR/Full",
+ [ETHTOOL_LINK_MODE_25000baseSR_Full_BIT] = "25000baseSR/Full",
+ [ETHTOOL_LINK_MODE_50000baseCR2_Full_BIT] = "50000baseCR2/Full",
+ [ETHTOOL_LINK_MODE_50000baseKR2_Full_BIT] = "50000baseKR2/Full",
+ [ETHTOOL_LINK_MODE_100000baseKR4_Full_BIT] = "100000baseKR4/Full",
+ [ETHTOOL_LINK_MODE_100000baseSR4_Full_BIT] = "100000baseSR4/Full",
+ [ETHTOOL_LINK_MODE_100000baseCR4_Full_BIT] = "100000baseCR4/Full",
+ [ETHTOOL_LINK_MODE_100000baseLR4_ER4_Full_BIT] = "100000baseLR4/ER4_Full",
+ [ETHTOOL_LINK_MODE_50000baseSR2_Full_BIT] = "50000baseSR2/Full",
+ [ETHTOOL_LINK_MODE_1000baseX_Full_BIT] = "1000baseX/Full",
+ [ETHTOOL_LINK_MODE_10000baseCR_Full_BIT] = "10000baseCR/Full",
+ [ETHTOOL_LINK_MODE_10000baseSR_Full_BIT] = "10000baseSR/Full",
+ [ETHTOOL_LINK_MODE_10000baseLR_Full_BIT] = "10000baseLR/Full",
+ [ETHTOOL_LINK_MODE_10000baseLRM_Full_BIT] = "10000baseLRM/Full",
+ [ETHTOOL_LINK_MODE_10000baseER_Full_BIT] = "10000baseER/Full",
+ [ETHTOOL_LINK_MODE_2500baseT_Full_BIT] = "2500baseT/Full",
+ [ETHTOOL_LINK_MODE_5000baseT_Full_BIT] = "5000baseT/Full",
+ [ETHTOOL_LINK_MODE_FEC_NONE_BIT] = "None",
+ [ETHTOOL_LINK_MODE_FEC_RS_BIT] = "RS",
+ [ETHTOOL_LINK_MODE_FEC_BASER_BIT] = "BASER",
+};
+
static const struct nla_policy dev_policy[ETHA_DEV_MAX + 1] = {
[ETHA_DEV_UNSPEC] = { .type = NLA_REJECT },
[ETHA_DEV_INDEX] = { .type = NLA_U32 },
@@ -541,6 +596,9 @@ static int __init ethnl_init(void)
{
int ret;
+ BUILD_BUG_ON(ARRAY_SIZE(link_mode_names) <
+ __ETHTOOL_LINK_MODE_MASK_NBITS);
+
ret = genl_register_family(ðtool_genl_family);
if (ret < 0)
panic("ethtool: could not register genetlink family\n");
diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h
index 82a4c1f398d8..800ea57ab0de 100644
--- a/net/ethtool/netlink.h
+++ b/net/ethtool/netlink.h
@@ -23,6 +23,7 @@ extern struct genl_family ethtool_genl_family;
extern const char *const so_timestamping_labels[];
extern const char *const tstamp_tx_type_labels[];
extern const char *const tstamp_rx_filter_labels[];
+extern const char *const link_mode_names[];
struct net_device *ethnl_dev_get(struct genl_info *info, struct nlattr *nest);
int ethnl_fill_dev(struct sk_buff *msg, struct net_device *dev, u16 attrtype);
diff --git a/net/ethtool/strset.c b/net/ethtool/strset.c
index 5c74498d9c72..dd87d7db8b61 100644
--- a/net/ethtool/strset.c
+++ b/net/ethtool/strset.c
@@ -85,6 +85,12 @@ static const struct strset_info info_template[] = {
.count = __HWTSTAMP_FILTER_COUNT,
.data = { .simple = tstamp_rx_filter_labels },
},
+ [ETH_SS_LINK_MODES] = {
+ .type = ETH_SS_TYPE_SIMPLE,
+ .per_dev = false,
+ .count = __ETHTOOL_LINK_MODE_MASK_NBITS,
+ .data = { .simple = link_mode_names },
+ },
};
struct strset_data {
--
2.20.1
^ permalink raw reply related
* [RFC PATCH net-next v3 09/21] ethtool: move string arrays into common file
From: Michal Kubecek @ 2019-02-18 18:22 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Andrew Lunn, Jakub Kicinski, Jiri Pirko,
linux-kernel
In-Reply-To: <cover.1550513384.git.mkubecek@suse.cz>
Introduce file net/ethtool/common.c for code shared by ioctl and netlink
ethtool interface. Move name tables of features, RSS hash functions,
tunables and PHY tunables into this file.
Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
---
net/ethtool/Makefile | 2 +-
net/ethtool/common.c | 83 ++++++++++++++++++++++++++++++++++++++++++++
net/ethtool/common.h | 17 +++++++++
net/ethtool/ioctl.c | 82 ++-----------------------------------------
4 files changed, 103 insertions(+), 81 deletions(-)
create mode 100644 net/ethtool/common.c
create mode 100644 net/ethtool/common.h
diff --git a/net/ethtool/Makefile b/net/ethtool/Makefile
index 482fdb9380fa..11782306593b 100644
--- a/net/ethtool/Makefile
+++ b/net/ethtool/Makefile
@@ -1,6 +1,6 @@
# SPDX-License-Identifier: GPL-2.0
-obj-y += ioctl.o
+obj-y += ioctl.o common.o
obj-$(CONFIG_ETHTOOL_NETLINK) += ethtool_nl.o
diff --git a/net/ethtool/common.c b/net/ethtool/common.c
new file mode 100644
index 000000000000..73f721a1c557
--- /dev/null
+++ b/net/ethtool/common.c
@@ -0,0 +1,83 @@
+// SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note
+
+#include "common.h"
+
+const char netdev_features_strings[NETDEV_FEATURE_COUNT][ETH_GSTRING_LEN] = {
+ [NETIF_F_SG_BIT] = "tx-scatter-gather",
+ [NETIF_F_IP_CSUM_BIT] = "tx-checksum-ipv4",
+ [NETIF_F_HW_CSUM_BIT] = "tx-checksum-ip-generic",
+ [NETIF_F_IPV6_CSUM_BIT] = "tx-checksum-ipv6",
+ [NETIF_F_HIGHDMA_BIT] = "highdma",
+ [NETIF_F_FRAGLIST_BIT] = "tx-scatter-gather-fraglist",
+ [NETIF_F_HW_VLAN_CTAG_TX_BIT] = "tx-vlan-hw-insert",
+
+ [NETIF_F_HW_VLAN_CTAG_RX_BIT] = "rx-vlan-hw-parse",
+ [NETIF_F_HW_VLAN_CTAG_FILTER_BIT] = "rx-vlan-filter",
+ [NETIF_F_HW_VLAN_STAG_TX_BIT] = "tx-vlan-stag-hw-insert",
+ [NETIF_F_HW_VLAN_STAG_RX_BIT] = "rx-vlan-stag-hw-parse",
+ [NETIF_F_HW_VLAN_STAG_FILTER_BIT] = "rx-vlan-stag-filter",
+ [NETIF_F_VLAN_CHALLENGED_BIT] = "vlan-challenged",
+ [NETIF_F_GSO_BIT] = "tx-generic-segmentation",
+ [NETIF_F_LLTX_BIT] = "tx-lockless",
+ [NETIF_F_NETNS_LOCAL_BIT] = "netns-local",
+ [NETIF_F_GRO_BIT] = "rx-gro",
+ [NETIF_F_GRO_HW_BIT] = "rx-gro-hw",
+ [NETIF_F_LRO_BIT] = "rx-lro",
+
+ [NETIF_F_TSO_BIT] = "tx-tcp-segmentation",
+ [NETIF_F_GSO_ROBUST_BIT] = "tx-gso-robust",
+ [NETIF_F_TSO_ECN_BIT] = "tx-tcp-ecn-segmentation",
+ [NETIF_F_TSO_MANGLEID_BIT] = "tx-tcp-mangleid-segmentation",
+ [NETIF_F_TSO6_BIT] = "tx-tcp6-segmentation",
+ [NETIF_F_FSO_BIT] = "tx-fcoe-segmentation",
+ [NETIF_F_GSO_GRE_BIT] = "tx-gre-segmentation",
+ [NETIF_F_GSO_GRE_CSUM_BIT] = "tx-gre-csum-segmentation",
+ [NETIF_F_GSO_IPXIP4_BIT] = "tx-ipxip4-segmentation",
+ [NETIF_F_GSO_IPXIP6_BIT] = "tx-ipxip6-segmentation",
+ [NETIF_F_GSO_UDP_TUNNEL_BIT] = "tx-udp_tnl-segmentation",
+ [NETIF_F_GSO_UDP_TUNNEL_CSUM_BIT] = "tx-udp_tnl-csum-segmentation",
+ [NETIF_F_GSO_PARTIAL_BIT] = "tx-gso-partial",
+ [NETIF_F_GSO_SCTP_BIT] = "tx-sctp-segmentation",
+ [NETIF_F_GSO_ESP_BIT] = "tx-esp-segmentation",
+ [NETIF_F_GSO_UDP_L4_BIT] = "tx-udp-segmentation",
+
+ [NETIF_F_FCOE_CRC_BIT] = "tx-checksum-fcoe-crc",
+ [NETIF_F_SCTP_CRC_BIT] = "tx-checksum-sctp",
+ [NETIF_F_FCOE_MTU_BIT] = "fcoe-mtu",
+ [NETIF_F_NTUPLE_BIT] = "rx-ntuple-filter",
+ [NETIF_F_RXHASH_BIT] = "rx-hashing",
+ [NETIF_F_RXCSUM_BIT] = "rx-checksum",
+ [NETIF_F_NOCACHE_COPY_BIT] = "tx-nocache-copy",
+ [NETIF_F_LOOPBACK_BIT] = "loopback",
+ [NETIF_F_RXFCS_BIT] = "rx-fcs",
+ [NETIF_F_RXALL_BIT] = "rx-all",
+ [NETIF_F_HW_L2FW_DOFFLOAD_BIT] = "l2-fwd-offload",
+ [NETIF_F_HW_TC_BIT] = "hw-tc-offload",
+ [NETIF_F_HW_ESP_BIT] = "esp-hw-offload",
+ [NETIF_F_HW_ESP_TX_CSUM_BIT] = "esp-tx-csum-hw-offload",
+ [NETIF_F_RX_UDP_TUNNEL_PORT_BIT] = "rx-udp_tunnel-port-offload",
+ [NETIF_F_HW_TLS_RECORD_BIT] = "tls-hw-record",
+ [NETIF_F_HW_TLS_TX_BIT] = "tls-hw-tx-offload",
+ [NETIF_F_HW_TLS_RX_BIT] = "tls-hw-rx-offload",
+};
+
+const char
+rss_hash_func_strings[ETH_RSS_HASH_FUNCS_COUNT][ETH_GSTRING_LEN] = {
+ [ETH_RSS_HASH_TOP_BIT] = "toeplitz",
+ [ETH_RSS_HASH_XOR_BIT] = "xor",
+ [ETH_RSS_HASH_CRC32_BIT] = "crc32",
+};
+
+const char
+tunable_strings[__ETHTOOL_TUNABLE_COUNT][ETH_GSTRING_LEN] = {
+ [ETHTOOL_ID_UNSPEC] = "Unspec",
+ [ETHTOOL_RX_COPYBREAK] = "rx-copybreak",
+ [ETHTOOL_TX_COPYBREAK] = "tx-copybreak",
+ [ETHTOOL_PFC_PREVENTION_TOUT] = "pfc-prevention-tout",
+};
+
+const char
+phy_tunable_strings[__ETHTOOL_PHY_TUNABLE_COUNT][ETH_GSTRING_LEN] = {
+ [ETHTOOL_ID_UNSPEC] = "Unspec",
+ [ETHTOOL_PHY_DOWNSHIFT] = "phy-downshift",
+};
diff --git a/net/ethtool/common.h b/net/ethtool/common.h
new file mode 100644
index 000000000000..41b2efc1e4e1
--- /dev/null
+++ b/net/ethtool/common.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+
+#ifndef _ETHTOOL_COMMON_H
+#define _ETHTOOL_COMMON_H
+
+#include <linux/ethtool.h>
+
+extern const char
+netdev_features_strings[NETDEV_FEATURE_COUNT][ETH_GSTRING_LEN];
+extern const char
+rss_hash_func_strings[ETH_RSS_HASH_FUNCS_COUNT][ETH_GSTRING_LEN];
+extern const char
+tunable_strings[__ETHTOOL_TUNABLE_COUNT][ETH_GSTRING_LEN];
+extern const char
+phy_tunable_strings[__ETHTOOL_PHY_TUNABLE_COUNT][ETH_GSTRING_LEN];
+
+#endif /* _ETHTOOL_COMMON_H */
diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index 1320e8dce559..71a1643adb2b 100644
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -31,6 +31,8 @@
#include <net/xdp_sock.h>
#include <net/flow_offload.h>
+#include "common.h"
+
/*
* Some useful ethtool_ops methods that're device independent.
* If we find that all drivers want to do the same thing here,
@@ -58,86 +60,6 @@ EXPORT_SYMBOL(ethtool_op_get_ts_info);
#define ETHTOOL_DEV_FEATURE_WORDS ((NETDEV_FEATURE_COUNT + 31) / 32)
-static const char netdev_features_strings[NETDEV_FEATURE_COUNT][ETH_GSTRING_LEN] = {
- [NETIF_F_SG_BIT] = "tx-scatter-gather",
- [NETIF_F_IP_CSUM_BIT] = "tx-checksum-ipv4",
- [NETIF_F_HW_CSUM_BIT] = "tx-checksum-ip-generic",
- [NETIF_F_IPV6_CSUM_BIT] = "tx-checksum-ipv6",
- [NETIF_F_HIGHDMA_BIT] = "highdma",
- [NETIF_F_FRAGLIST_BIT] = "tx-scatter-gather-fraglist",
- [NETIF_F_HW_VLAN_CTAG_TX_BIT] = "tx-vlan-hw-insert",
-
- [NETIF_F_HW_VLAN_CTAG_RX_BIT] = "rx-vlan-hw-parse",
- [NETIF_F_HW_VLAN_CTAG_FILTER_BIT] = "rx-vlan-filter",
- [NETIF_F_HW_VLAN_STAG_TX_BIT] = "tx-vlan-stag-hw-insert",
- [NETIF_F_HW_VLAN_STAG_RX_BIT] = "rx-vlan-stag-hw-parse",
- [NETIF_F_HW_VLAN_STAG_FILTER_BIT] = "rx-vlan-stag-filter",
- [NETIF_F_VLAN_CHALLENGED_BIT] = "vlan-challenged",
- [NETIF_F_GSO_BIT] = "tx-generic-segmentation",
- [NETIF_F_LLTX_BIT] = "tx-lockless",
- [NETIF_F_NETNS_LOCAL_BIT] = "netns-local",
- [NETIF_F_GRO_BIT] = "rx-gro",
- [NETIF_F_GRO_HW_BIT] = "rx-gro-hw",
- [NETIF_F_LRO_BIT] = "rx-lro",
-
- [NETIF_F_TSO_BIT] = "tx-tcp-segmentation",
- [NETIF_F_GSO_ROBUST_BIT] = "tx-gso-robust",
- [NETIF_F_TSO_ECN_BIT] = "tx-tcp-ecn-segmentation",
- [NETIF_F_TSO_MANGLEID_BIT] = "tx-tcp-mangleid-segmentation",
- [NETIF_F_TSO6_BIT] = "tx-tcp6-segmentation",
- [NETIF_F_FSO_BIT] = "tx-fcoe-segmentation",
- [NETIF_F_GSO_GRE_BIT] = "tx-gre-segmentation",
- [NETIF_F_GSO_GRE_CSUM_BIT] = "tx-gre-csum-segmentation",
- [NETIF_F_GSO_IPXIP4_BIT] = "tx-ipxip4-segmentation",
- [NETIF_F_GSO_IPXIP6_BIT] = "tx-ipxip6-segmentation",
- [NETIF_F_GSO_UDP_TUNNEL_BIT] = "tx-udp_tnl-segmentation",
- [NETIF_F_GSO_UDP_TUNNEL_CSUM_BIT] = "tx-udp_tnl-csum-segmentation",
- [NETIF_F_GSO_PARTIAL_BIT] = "tx-gso-partial",
- [NETIF_F_GSO_SCTP_BIT] = "tx-sctp-segmentation",
- [NETIF_F_GSO_ESP_BIT] = "tx-esp-segmentation",
- [NETIF_F_GSO_UDP_L4_BIT] = "tx-udp-segmentation",
-
- [NETIF_F_FCOE_CRC_BIT] = "tx-checksum-fcoe-crc",
- [NETIF_F_SCTP_CRC_BIT] = "tx-checksum-sctp",
- [NETIF_F_FCOE_MTU_BIT] = "fcoe-mtu",
- [NETIF_F_NTUPLE_BIT] = "rx-ntuple-filter",
- [NETIF_F_RXHASH_BIT] = "rx-hashing",
- [NETIF_F_RXCSUM_BIT] = "rx-checksum",
- [NETIF_F_NOCACHE_COPY_BIT] = "tx-nocache-copy",
- [NETIF_F_LOOPBACK_BIT] = "loopback",
- [NETIF_F_RXFCS_BIT] = "rx-fcs",
- [NETIF_F_RXALL_BIT] = "rx-all",
- [NETIF_F_HW_L2FW_DOFFLOAD_BIT] = "l2-fwd-offload",
- [NETIF_F_HW_TC_BIT] = "hw-tc-offload",
- [NETIF_F_HW_ESP_BIT] = "esp-hw-offload",
- [NETIF_F_HW_ESP_TX_CSUM_BIT] = "esp-tx-csum-hw-offload",
- [NETIF_F_RX_UDP_TUNNEL_PORT_BIT] = "rx-udp_tunnel-port-offload",
- [NETIF_F_HW_TLS_RECORD_BIT] = "tls-hw-record",
- [NETIF_F_HW_TLS_TX_BIT] = "tls-hw-tx-offload",
- [NETIF_F_HW_TLS_RX_BIT] = "tls-hw-rx-offload",
-};
-
-static const char
-rss_hash_func_strings[ETH_RSS_HASH_FUNCS_COUNT][ETH_GSTRING_LEN] = {
- [ETH_RSS_HASH_TOP_BIT] = "toeplitz",
- [ETH_RSS_HASH_XOR_BIT] = "xor",
- [ETH_RSS_HASH_CRC32_BIT] = "crc32",
-};
-
-static const char
-tunable_strings[__ETHTOOL_TUNABLE_COUNT][ETH_GSTRING_LEN] = {
- [ETHTOOL_ID_UNSPEC] = "Unspec",
- [ETHTOOL_RX_COPYBREAK] = "rx-copybreak",
- [ETHTOOL_TX_COPYBREAK] = "tx-copybreak",
- [ETHTOOL_PFC_PREVENTION_TOUT] = "pfc-prevention-tout",
-};
-
-static const char
-phy_tunable_strings[__ETHTOOL_PHY_TUNABLE_COUNT][ETH_GSTRING_LEN] = {
- [ETHTOOL_ID_UNSPEC] = "Unspec",
- [ETHTOOL_PHY_DOWNSHIFT] = "phy-downshift",
-};
-
static int ethtool_get_features(struct net_device *dev, void __user *useraddr)
{
struct ethtool_gfeatures cmd = {
--
2.20.1
^ permalink raw reply related
* [RFC PATCH net-next v3 08/21] ethtool: generic handlers for GET requests
From: Michal Kubecek @ 2019-02-18 18:22 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Andrew Lunn, Jakub Kicinski, Jiri Pirko,
linux-kernel
In-Reply-To: <cover.1550513384.git.mkubecek@suse.cz>
Some parts of processing GET type requests and related notifications are
independent of a command. Provide universal functions so that only four
callbacks need to be defined for each command type:
parse_request() - parse incoming message
prepare_data() - retrieve data from driver or NIC
reply_size() - estimate reply message size
fill_reply() - compose reply message
These callback are defined in an instance of struct get_request_ops.
Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
---
net/ethtool/netlink.c | 286 ++++++++++++++++++++++++++++++++++++++++++
net/ethtool/netlink.h | 84 +++++++++++++
2 files changed, 370 insertions(+)
diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
index ee3424cd1f90..8cdb6f52cb4a 100644
--- a/net/ethtool/netlink.c
+++ b/net/ethtool/netlink.c
@@ -84,6 +84,41 @@ int ethnl_fill_dev(struct sk_buff *msg, struct net_device *dev, u16 attrtype)
return ret;
}
+/* GET request helpers */
+
+const struct get_request_ops *get_requests[__ETHNL_CMD_CNT] = {
+};
+
+static struct common_req_info *alloc_get_data(const struct get_request_ops *ops)
+{
+ struct common_req_info *req_info = kmalloc(ops->data_size, GFP_KERNEL);
+
+ if (!req_info)
+ return NULL;
+ memset(req_info, '\0', ops->repdata_offset);
+ req_info->reply_data =
+ (struct common_reply_data *)((char *)req_info +
+ ops->repdata_offset);
+ return req_info;
+}
+
+static void free_get_data(const struct get_request_ops *ops,
+ struct common_req_info *req_info)
+{
+ if (ops->cleanup)
+ ops->cleanup(req_info);
+ kfree(req_info);
+}
+
+static void init_reply_data(const struct common_req_info *req_info,
+ const struct get_request_ops *ops,
+ struct net_device *dev)
+{
+ memset(req_info->reply_data, '\0',
+ ops->data_size - ops->repdata_offset);
+ req_info->reply_data->dev = dev;
+}
+
/* create skb for a reply and fill device identification
* payload: payload length (without netlink and genetlink header)
* dev: device the reply is about (may be null)
@@ -124,6 +159,257 @@ struct sk_buff *ethnl_reply_init(size_t payload, struct net_device *dev, u8 cmd,
return NULL;
}
+int ethnl_get_doit(struct sk_buff *skb, struct genl_info *info)
+{
+ const u8 cmd = info->genlhdr->cmd;
+ struct common_req_info *req_info;
+ const struct get_request_ops *ops;
+ struct sk_buff *rskb;
+ void *reply_payload;
+ int reply_len;
+ int ret;
+
+ ops = get_requests[cmd];
+ if (WARN_ONCE(!ops, "cmd %u has no get_request_ops\n", cmd))
+ return -EOPNOTSUPP;
+ req_info = alloc_get_data(ops);
+ if (!req_info)
+ return -ENOMEM;
+ ret = ops->parse_request(req_info, skb, info, info->nlhdr);
+ if (!ops->allow_nodev_do && !req_info->dev) {
+ ETHNL_SET_ERRMSG(info, "device not specified in do request");
+ ret = -EINVAL;
+ }
+ if (ret < 0)
+ goto err_dev;
+ init_reply_data(req_info, ops, req_info->dev);
+
+ rtnl_lock();
+ ret = ops->prepare_data(req_info, info);
+ if (ret < 0)
+ goto err_rtnl;
+ reply_len = ops->reply_size(req_info);
+ if (ret < 0)
+ goto err_rtnl;
+ ret = -ENOMEM;
+ rskb = ethnl_reply_init(reply_len, req_info->dev, ops->reply_cmd,
+ ops->dev_attrtype, info, &reply_payload);
+ if (!rskb)
+ goto err_rtnl;
+ ret = ops->fill_reply(rskb, req_info);
+ if (ret < 0)
+ goto err;
+ rtnl_unlock();
+
+ genlmsg_end(rskb, reply_payload);
+ if (req_info->dev)
+ dev_put(req_info->dev);
+ free_get_data(ops, req_info);
+ return genlmsg_reply(rskb, info);
+
+err:
+ WARN_ONCE(ret == -EMSGSIZE,
+ "calculated message payload length (%d) not sufficient\n",
+ reply_len);
+ nlmsg_free(rskb);
+ free_get_data(ops, req_info);
+err_rtnl:
+ rtnl_unlock();
+err_dev:
+ if (req_info->dev)
+ dev_put(req_info->dev);
+ return ret;
+}
+
+static int ethnl_get_dump_one(struct sk_buff *skb,
+ struct net_device *dev,
+ const struct get_request_ops *ops,
+ struct common_req_info *req_info)
+{
+ int ret;
+
+ init_reply_data(req_info, ops, dev);
+ rtnl_lock();
+ ret = ops->prepare_data(req_info, NULL);
+ if (ret < 0)
+ return ret;
+ ret = ethnl_fill_dev(skb, dev, ops->dev_attrtype);
+ if (ret < 0)
+ return ret;
+ ret = ops->fill_reply(skb, req_info);
+ rtnl_unlock();
+
+ req_info->reply_data->dev = NULL;
+ return ret;
+}
+
+/* generic ->dumpit() handler; device iteration copied from rtnl_dump_ifinfo()
+ * cb->args[0]: pointer to struct get_request_ops
+ * cb->args[1]: pointer to request data
+ * cb->args[2]: iteration position - hashbucket
+ * cb->args[3]: iteration position - ifindex
+ */
+int ethnl_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
+{
+ struct net *net = sock_net(skb->sk);
+ struct common_req_info *req_info;
+ const struct get_request_ops *ops;
+ int h, s_h, idx = 0, s_idx;
+ struct hlist_head *head;
+ struct net_device *dev;
+ int ret = 0;
+ void *ehdr;
+
+ ops = (const struct get_request_ops *)cb->args[0];
+ req_info = (struct common_req_info *)cb->args[1];
+ s_h = cb->args[2];
+ s_idx = cb->args[3];
+
+ for (h = s_h; h < NETDEV_HASHENTRIES; h++, s_idx = 0) {
+ idx = 0;
+ head = &net->dev_index_head[h];
+ hlist_for_each_entry(dev, head, index_hlist) {
+ if (idx < s_idx)
+ goto cont;
+ ehdr = genlmsg_put(skb, NETLINK_CB(cb->skb).portid,
+ cb->nlh->nlmsg_seq,
+ ðtool_genl_family, 0,
+ ops->reply_cmd);
+ ret = ethnl_get_dump_one(skb, dev, ops, req_info);
+ if (ret < 0) {
+ genlmsg_cancel(skb, ehdr);
+ if (ret == -EOPNOTSUPP)
+ goto cont;
+ if (likely(skb->len))
+ goto out;
+ goto out_err;
+ }
+ genlmsg_end(skb, ehdr);
+cont:
+ idx++;
+ }
+ }
+out:
+ ret = skb->len;
+out_err:
+ cb->args[2] = h;
+ cb->args[3] = idx;
+ cb->seq = net->dev_base_seq;
+ nl_dump_check_consistent(cb, nlmsg_hdr(skb));
+
+ return ret;
+}
+
+/* generic ->start() handler for GET requests */
+static int ethnl_get_start(struct netlink_callback *cb)
+{
+ struct common_req_info *req_info;
+ const struct get_request_ops *ops;
+ struct genlmsghdr *ghdr;
+ int ret;
+
+ ghdr = nlmsg_data(cb->nlh);
+ ops = get_requests[ghdr->cmd];
+ if (WARN_ONCE(!ops, "cmd %u has no get_request_ops\n", ghdr->cmd))
+ return -EOPNOTSUPP;
+ req_info = alloc_get_data(ops);
+ if (!req_info)
+ return -ENOMEM;
+
+ ret = ops->parse_request(req_info, cb->skb, NULL, cb->nlh);
+ if (req_info->dev) {
+ /* We ignore device specification in dump requests but as the
+ * same parser as for non-dump (doit) requests is used, it
+ * would take reference to the device if it finds one
+ */
+ dev_put(req_info->dev);
+ req_info->dev = NULL;
+ }
+ if (ret < 0)
+ return ret;
+
+ cb->args[0] = (long)ops;
+ cb->args[1] = (long)req_info;
+ cb->args[2] = 0;
+ cb->args[3] = 0;
+
+ return 0;
+}
+
+/* generic ->done() handler for GET requests */
+static int ethnl_get_done(struct netlink_callback *cb)
+{
+ free_get_data((const struct get_request_ops *)cb->args[0],
+ (struct common_req_info *)cb->args[1]);
+
+ return 0;
+}
+
+/* generic notification handler */
+static void ethnl_std_notify(struct net_device *dev,
+ struct netlink_ext_ack *extack, unsigned int cmd,
+ u32 req_mask, const void *data)
+{
+ struct common_req_info *req_info;
+ const struct get_request_ops *ops;
+ struct sk_buff *skb;
+ void *reply_payload;
+ int reply_len;
+ int ret;
+
+ ops = get_requests[cmd - 1];
+ if (WARN_ONCE(!ops, "cmd %u has no get_request_ops\n", cmd - 1))
+ return;
+ /* when ethnl_std_notify() is used as notify handler, command id of
+ * corresponding GET request must be one less than cmd argument passed
+ * to ethnl_std_notify()
+ */
+ if (WARN_ONCE(ops->reply_cmd != cmd,
+ "reply_cmd for %u is %u, expected %u\n", cmd - 1,
+ ops->reply_cmd, cmd))
+ return;
+
+ req_info = alloc_get_data(ops);
+ if (!req_info)
+ return;
+ req_info->dev = dev;
+ req_info->req_mask = req_mask;
+ req_info->compact = true;
+
+ init_reply_data(req_info, ops, dev);
+ ret = ops->prepare_data(req_info, NULL);
+ if (ret < 0)
+ goto err_data;
+ reply_len = ops->reply_size(req_info);
+ if (reply_len < 0)
+ goto err_data;
+ skb = genlmsg_new(reply_len, GFP_KERNEL);
+ if (!skb)
+ goto err_data;
+ reply_payload = genlmsg_put(skb, 0, ++ethnl_bcast_seq,
+ ðtool_genl_family, 0, ops->reply_cmd);
+ if (!reply_payload)
+ goto err_skb;
+
+ ret = ethnl_fill_dev(skb, dev, ops->dev_attrtype);
+ if (ret < 0)
+ goto err_skb;
+ ret = ops->fill_reply(skb, req_info);
+ if (ret < 0)
+ goto err_skb;
+ free_get_data(ops, req_info);
+ genlmsg_end(skb, reply_payload);
+
+ genlmsg_multicast(ðtool_genl_family, skb, 0, ETHNL_MCGRP_MONITOR,
+ GFP_KERNEL);
+ return;
+
+err_skb:
+ nlmsg_free(skb);
+err_data:
+ free_get_data(ops, req_info);
+}
+
/* notifications */
typedef void (*ethnl_notify_handler_t)(struct net_device *dev,
diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h
index 78385baeaec0..7141ec71a6d3 100644
--- a/net/ethtool/netlink.h
+++ b/net/ethtool/netlink.h
@@ -155,4 +155,88 @@ static inline unsigned int dev_ident_size(void)
nla_total_size(IFNAMSIZ));
}
+/* GET request handling */
+
+struct common_reply_data;
+
+/* Structure holding data for unified processing a GET request consists of two
+ * parts: request info and reply data. Request info starts at offset 0 with
+ * embedded struct common_req_info, is usually filled by ->parse_request()
+ * and is common for all reply messages to one request. Reply data start with
+ * embedded struct common_reply_data and contain data specific to a reply
+ * message (usually one per device for dump requests); this part is filled by
+ * ->prepare_data()
+ *
+ * @reply_data: pointer to corresponding struct common_reply_data
+ * @dev: requested device; may be null (dumps), if not, reference is held
+ * @req_mask: bit mask of parts of information requested
+ * @compact: use compact format for bitsets
+ */
+struct common_req_info {
+ struct common_reply_data *reply_data;
+ struct net_device *dev;
+ u32 req_mask;
+ bool compact;
+};
+
+/* @dev: device for current reply message
+ * @info_mask: subset of req_mask (without information which is not available)
+ */
+struct common_reply_data {
+ struct net_device *dev;
+ u32 info_mask;
+};
+
+static inline int ethnl_before_ops(struct net_device *dev)
+{
+ if (dev && dev->ethtool_ops->begin)
+ return dev->ethtool_ops->begin(dev);
+ else
+ return 0;
+}
+
+static inline void ethnl_after_ops(struct net_device *dev)
+{
+ if (dev && dev->ethtool_ops->complete)
+ dev->ethtool_ops->complete(dev);
+}
+
+/* parameters and callbacks for unified handling of GET requests
+ * @request_cmd: command id for request (GET)
+ * @reply_cmd: command id for reply (SET)
+ * @dev_attr: attr type for device specification
+ * @data_size: total length of data structure
+ * @repdata_offset: offset of "reply data" part (struct common_reply_data)
+ * @allow_nodev_do: do not fail if device is not specified for non-dump request
+ * @parse_request: parse request message and fill request info; request info
+ * is zero initialized on entry except reply_data pointer (which is set)
+ * @prepare_data: retrieve data needed to compose a reply message; reply data
+ * is zero initialized on entry except @dev
+ * @reply_size: return size of reply message payload without device
+ * specification; reported size may be slightly bigger than actual reply
+ * but must not be smaller
+ * @fill_reply: fill reply message payload
+ * @cleanup: (optional) called when data are no longer needed; use e.g. to free
+ * any additional data allocated in prepare_data() which are not part
+ * of the main structure
+ */
+struct get_request_ops {
+ u8 request_cmd;
+ u8 reply_cmd;
+ u16 dev_attrtype;
+ unsigned int data_size;
+ unsigned int repdata_offset;
+ bool allow_nodev_do;
+
+ int (*parse_request)(struct common_req_info *req_info,
+ struct sk_buff *skb, struct genl_info *info,
+ const struct nlmsghdr *nlhdr);
+ int (*prepare_data)(struct common_req_info *req_info,
+ struct genl_info *info);
+ int (*reply_size)(const struct common_req_info *req_info);
+ int (*fill_reply)(struct sk_buff *skb,
+ const struct common_req_info *req_info);
+ void (*cleanup)(struct common_req_info *req_info);
+};
+
#endif /* _NET_ETHTOOL_NETLINK_H */
--
2.20.1
^ permalink raw reply related
* [RFC PATCH net-next v3 01/21] netlink: introduce nla_put_bitfield32()
From: Michal Kubecek @ 2019-02-18 18:21 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Andrew Lunn, Jakub Kicinski, Jiri Pirko,
linux-kernel
In-Reply-To: <cover.1550513384.git.mkubecek@suse.cz>
Similar to other data types, this helper puts NLA_BITFIELD32 attribute into
a netlink message. It takes separate value and selector arguments, if there
is a struct nla_bitfield32 already, one can use nla_put().
Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
---
include/net/netlink.h | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/include/net/netlink.h b/include/net/netlink.h
index 23f27b0b3cef..bc0497076bec 100644
--- a/include/net/netlink.h
+++ b/include/net/netlink.h
@@ -1211,6 +1211,21 @@ static inline int nla_put_in6_addr(struct sk_buff *skb, int attrtype,
return nla_put(skb, attrtype, sizeof(*addr), addr);
}
+/**
+ * nla_put_bitfield32 - Add a bitfield32 value/selector attribute to
+ * a socket buffer
+ * @skb: socket buffer to add attribute to
+ * @value: 32-bit value bitmap
+ * @selector: 32-bit selector bitmap
+ */
+static inline int nla_put_bitfield32(struct sk_buff *skb, int attrtype,
+ u32 value, u32 selector)
+{
+ struct nla_bitfield32 tmp = { .value = value, .selector = selector };
+
+ return nla_put(skb, attrtype, sizeof(tmp), &tmp);
+}
+
/**
* nla_get_u32 - return payload of u32 attribute
* @nla: u32 netlink attribute
--
2.20.1
^ permalink raw reply related
* [RFC PATCH net-next v3 00/21] ethtool netlink interface, part 1
From: Michal Kubecek @ 2019-02-18 18:21 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Andrew Lunn, Jakub Kicinski, Jiri Pirko,
linux-kernel
Note: this is marked as RFC because it's rather late in the cycle; the plan
is to make a regular submission (with changes based on review) once
net-next reopens after the 5.1 merge window. The full (work in progress)
series, together with the (userspace) ethtool counterpart can be found at
https://github.com/mkubecek/ethnl
This is first part of alternative userspace interface for ethtool. The aim
is to address some long known issues with the ioctl interface, mainly lack
of extensibility, raciness, limited error reporting and absence of
notifications.
The interface uses generic netlink family "ethtool"; it provides multicast
group "monitor" which is used for notifications. Documentation for the
interface is in Documentation/networking/ethtool-netlink.txt
Basic concepts:
- the goal is to provide all features of ioctl interface but allow
easier future extensions; at some point, it should be possible to have
full ethtool functionality without using the ioctl interface
- inextensibility of ioctl interface resulted in way too many commands,
many of them obsoleted by newer ones; reduce the number by ignoring the
obsolete commands and grouping some together
- for "set" type commands, allows passing only the attributes to be
changed; therefore we don't need a get-modify-set cycle (which is
inherently racy), userspace can simply say what it wants to change
- provide notifications to multicast group "monitor" like rtnetlink does,
i.e. in the form of messages close to replies to "get" requests
- allow dump requests to get some information about all network devices
providing it
- be less dependent on ethtool and kernel being in sync; allow e.g. saying
"ethtool -s eth0 advertise foo off" without ethtool knowing what "foo"
means; it's kernel's job to know what mode "xyz" is and if it exists and
is supported
Main changes between RFC v2 and RFC v3:
- do not allow building as a module (no netdev notifiers needed)
- drop some obsolete fields
- add permanent hw address, timestamping and private flags support
- rework bitset handling to get rid of variable length arrays
- notify monitor on device renames
- restructure GET_SETTINGS/SET_SETTINGS messages
- split too long patches and submit only first part of the series
Main changes between RFC v1 and RFC v2:
- support dumps for all "get" requests
- provide notifications for changes related to supported request types
- support getting string sets (both global and per device)
- support getting/setting device features
- get rid of family specific header, everything passed as attributes
- split netlink code into multiple files in net/ethtool/ directory
ToDo / open questions:
- some features provided by ethtool would rather belong to devlink (and
some are already superseded by devlink); however, only few drivers
provide devlink interface at the moment and as recent discussion on
flashing revealed, we cannot rely on devlink's presence
- while the netlink interface allows easy future extensions, ethtool_ops
interface does not; some settings could be implemented using tunables and
accessed via relevant netlink messages (as well as tunables) from
userspace but in the long term, something better will be needed
- currently, all communication with drivers via ethtool_ops is done
under RTNL as this is what ioctl interface does and likely many
ethtool_ops handlers rely on that; if we are going to rework ethtool_ops
in the future ("phase two"), it would be nice to get rid of it
- ethtool_ops should pass extack pointer to allow drivers more meaningful
error reporting; it's not clear, however, how to pass information about
offending attribute
- notifications are sent whenever a change is done via netlink API or
ioctl API and for netdev features also whenever they are updated using
netdev_change_features(); it would be desirable to notify also about
link state and negotiation result (speed/duplex and partner link
modes) but it would be more tricky
Michal Kubecek (21):
netlink: introduce nla_put_bitfield32()
ethtool: move to its own directory
ethtool: introduce ethtool netlink interface
ethtool: helper functions for netlink interface
ethtool: netlink bitset handling
ethtool: support for netlink notifications
ethtool: implement EVENT notifications
ethtool: generic handlers for GET requests
ethtool: move string arrays into common file
ethtool: provide string sets with GET_STRSET request
ethtool: provide driver/device information in GET_INFO request
ethtool: provide permanent hardware address in GET_INFO request
ethtool: provide timestamping information in GET_INFO request
ethtool: provide link mode names as a string set
ethtool: provide link settings and link modes in GET_SETTINGS request
ethtool: provide WoL information in GET_SETTINGS request
ethtool: provide message level in GET_SETTINGS request
ethtool: provide link state in GET_SETTINGS request
ethtool: provide device features in GET_SETTINGS request
ethtool: provide private flags in GET_SETTINGS request
ethtool: send netlink notifications about setting changes
Documentation/networking/ethtool-netlink.txt | 441 +++++++++++++
include/linux/ethtool_netlink.h | 17 +
include/linux/netdevice.h | 14 +
include/net/netlink.h | 15 +
include/uapi/linux/ethtool.h | 10 +
include/uapi/linux/ethtool_netlink.h | 265 ++++++++
include/uapi/linux/net_tstamp.h | 13 +
net/Kconfig | 8 +
net/Makefile | 2 +-
net/core/Makefile | 2 +-
net/ethtool/Makefile | 7 +
net/ethtool/bitset.c | 572 +++++++++++++++++
net/ethtool/bitset.h | 40 ++
net/ethtool/common.c | 227 +++++++
net/ethtool/common.h | 29 +
net/ethtool/info.c | 332 ++++++++++
net/{core/ethtool.c => ethtool/ioctl.c} | 244 ++-----
net/ethtool/netlink.c | 634 +++++++++++++++++++
net/ethtool/netlink.h | 252 ++++++++
net/ethtool/settings.c | 559 ++++++++++++++++
net/ethtool/strset.c | 461 ++++++++++++++
21 files changed, 3937 insertions(+), 207 deletions(-)
create mode 100644 Documentation/networking/ethtool-netlink.txt
create mode 100644 include/linux/ethtool_netlink.h
create mode 100644 include/uapi/linux/ethtool_netlink.h
create mode 100644 net/ethtool/Makefile
create mode 100644 net/ethtool/bitset.c
create mode 100644 net/ethtool/bitset.h
create mode 100644 net/ethtool/common.c
create mode 100644 net/ethtool/common.h
create mode 100644 net/ethtool/info.c
rename net/{core/ethtool.c => ethtool/ioctl.c} (91%)
create mode 100644 net/ethtool/netlink.c
create mode 100644 net/ethtool/netlink.h
create mode 100644 net/ethtool/settings.c
create mode 100644 net/ethtool/strset.c
--
2.20.1
^ permalink raw reply
* Re: [PATCH net-next v4 05/17] net: sched: traverse chains in block with tcf_get_next_chain()
From: Cong Wang @ 2019-02-18 18:26 UTC (permalink / raw)
To: Vlad Buslov
Cc: Linux Kernel Network Developers, Jamal Hadi Salim, Jiri Pirko,
David Miller, Alexei Starovoitov, Daniel Borkmann
In-Reply-To: <vbflg2dqukb.fsf@mellanox.com>
On Mon, Feb 18, 2019 at 2:07 AM Vlad Buslov <vladbu@mellanox.com> wrote:
>
> Hi Cong,
>
> Thanks for reviewing!
>
> On Fri 15 Feb 2019 at 22:21, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> > (Sorry for joining this late.)
> >
> > On Mon, Feb 11, 2019 at 12:56 AM Vlad Buslov <vladbu@mellanox.com> wrote:
> >> @@ -2432,7 +2474,11 @@ static int tc_dump_chain(struct sk_buff *skb, struct netlink_callback *cb)
> >> index_start = cb->args[0];
> >> index = 0;
> >>
> >> - list_for_each_entry(chain, &block->chain_list, list) {
> >> + for (chain = __tcf_get_next_chain(block, NULL);
> >> + chain;
> >> + chain_prev = chain,
> >> + chain = __tcf_get_next_chain(block, chain),
> >> + tcf_chain_put(chain_prev)) {
> >
> > Why do you want to take the block->lock in each iteration
> > of the loop rather than taking once for the whole loop?
>
> This loop calls classifier ops callback in tc_chain_fill_node(). I don't
> call any classifier ops callbacks while holding block or chain lock in
> this change because the goal is to achieve fine-grained locking for data
> structures used by filter update path. Locking per-block or per-chain is
> much coarser than taking reference counters to parent structures and
> allowing classifiers to implement their own locking.
That is the problem, when we have N filter chains in a block, you
lock and unlock mutex N times... And what __tcf_get_next_chain()
does is basically just retrieving the next entry in the list, so the
overhead of mutex is likely more than the list operation itself in
contention situation.
Now I can see why you complained about mutex before, it is
how you use it, not actually its own problem. :)
>
> In this case call to ops->tmplt_dump() is probably quite fast and its
> execution time doesn't depend on number of filters on the classifier, so
> releasing block->lock on each iteration doesn't provide much benefit, if
> at all. However, it is easier for me to reason about locking correctness
> in this refactoring by following a simple rule that no locks (besides
> rtnl mutex) can be held when calling classifier ops callbacks.
Well, for me, a hierarchy locking is always simple when you take
them in the right order, that is locking the larger-scope lock first
and then smaller-scope one.
The way you use the locking here is actually harder for me to
review, because it is hard to valid its atomicity when you unlock
the larger scope lock and re-take the smaller scope lock. You
use refcnt to ensure it will not go way, but that is still far from
guarantee of the atomicity.
For example, tp->ops->change() which changes an existing
filter, I don't see you lock either block->lock or
chain->filter_chain_lock when calling it. How does it even work?
Thanks.
^ permalink raw reply
* Re: [PATCH net-next v4 07/17] net: sched: protect filter_chain list with filter_chain_lock mutex
From: Cong Wang @ 2019-02-18 18:31 UTC (permalink / raw)
To: Vlad Buslov
Cc: Linux Kernel Network Developers, Jamal Hadi Salim, Jiri Pirko,
David Miller, Alexei Starovoitov, Daniel Borkmann
In-Reply-To: <vbfk1hxqrxr.fsf@mellanox.com>
On Mon, Feb 18, 2019 at 3:06 AM Vlad Buslov <vladbu@mellanox.com> wrote:
>
> On Fri 15 Feb 2019 at 22:35, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> > On Mon, Feb 11, 2019 at 12:56 AM Vlad Buslov <vladbu@mellanox.com> wrote:
> >> +#ifdef CONFIG_PROVE_LOCKING
> >> +static inline bool lockdep_tcf_chain_is_locked(struct tcf_chain *chain)
> >> +{
> >> + return lockdep_is_held(&chain->filter_chain_lock);
> >> +}
> >> +#else
> >> +static inline bool lockdep_tcf_chain_is_locked(struct tcf_block *chain)
> >> +{
> >> + return true;
> >> +}
> >> +#endif /* #ifdef CONFIG_PROVE_LOCKING */
> >> +
> >> +#define tcf_chain_dereference(p, chain) \
> >> + rcu_dereference_protected(p, lockdep_tcf_chain_is_locked(chain))
> >
> >
> > Are you sure you need this #ifdef CONFIG_PROVE_LOCKING?
> > rcu_dereference_protected() should already test CONFIG_PROVE_RCU.
> >
> > Ditto for tcf_proto_dereference().
>
> I implemented these macro same way as rtnl_dereference() is implemented,
> which they are intended to substitute.
>
> After removing them I get following compilation error with
> CONFIG_PROVE_LOCKING disabled:
This is pretty odd, because net/core/neighbour.c uses it without
any #ifdef CONFIG_PROVE_LOCKING, for instance:
192 neigh = rcu_dereference_protected(n->next,
193
lockdep_is_held(&tbl->lock));
194 rcu_assign_pointer(*np, neigh);
195 neigh_mark_dead(n);
196 retval = true;
So how does this compile when CONFIG_PROVE_LOCKING
is disabled? :-/
^ permalink raw reply
* Re: TC stats / hw offload question
From: Edward Cree @ 2019-02-18 18:56 UTC (permalink / raw)
To: Jamal Hadi Salim, netdev
Cc: Jiri Pirko, Cong Wang, Or Gerlitz, Andy Gospodarek, PJ Waskiewicz,
Anjali Singhai Jain, Jakub Kicinski
In-Reply-To: <702dd5b7-c6ed-b669-8270-d44f5ff4fb30@mojatatu.com>
On 14/02/19 12:39, Jamal Hadi Salim wrote:
> On 2019-02-11 6:44 a.m., Edward Cree wrote:
>> My end goal is to implement TC offload in some hw we're designing
>> here at Solarflare. So I'm trying to determine what hardware is
>> expected/required to do.
>> It might be possible to design our new hw so that we can attach a
>> counter to every action, if that's what TC wants.
>
> It makes sense to have a counter on every action - even if it is
> for debugging purposes. The two most basic actions are "drop" or
> "accept". In TC speak the default action is "classid x:y" which
> typically is to select a queue or give the flow some identity
Perhaps I was insufficiently clear; we're only looking at the switching
side of things (e.g. OVS offload) right now; we don't yet have a plan
for 'delivery' filters (I imagine we'll probably initially port over
our ethtool ntuple filter handling from ef10, though we may end up
going down the TC route there).
So I think at the moment 'classid' isn't relevant (?)
> Note, your counters should also be shareable; example, count all
> the drops in one counter across multiple flows as in the following
> case where counter index 1 is used.
>
> tc flower match foo action drop index 1
> tc flower match bar action drop index 1
[...]
> allow them to specify
> the counter index (assuming you architecture has an indexed table
> of counters).
Our architecture allocates objects (including counters) and returns
opaque handles to them, so we'd need a software table to connect
counter index to FW counter ID.
Also, sharing counters in hw causes extra work for the driver code
that keeps track of which encap actions are getting hit (so it can
keep the neighbour entries alive). Maybe summing the shared
counters in sw is easier than that, I'm not sure (or maybe encap
action counters should just be kept separate from the counters we
report to TC).
TBH I'm coming to the conclusion that what we should do for the first
version of our driver is just to create a counter per rule and report
it against the first action (only), and for now ignore the index (or
maybe require it to be set to some distinguished value, like 0, to
mean 'allocate', so that as a future extension we can support
shareable counters).
-Ed
^ permalink raw reply
* Re: [PATCH net-next v4 17/17] net: sched: unlock rules update API
From: Cong Wang @ 2019-02-18 18:56 UTC (permalink / raw)
To: Vlad Buslov
Cc: Linux Kernel Network Developers, Jamal Hadi Salim, Jiri Pirko,
David Miller, Alexei Starovoitov, Daniel Borkmann
In-Reply-To: <20190211085548.7190-18-vladbu@mellanox.com>
On Mon, Feb 11, 2019 at 12:56 AM Vlad Buslov <vladbu@mellanox.com> wrote:
>
> Register netlink protocol handlers for message types RTM_NEWTFILTER,
> RTM_DELTFILTER, RTM_GETTFILTER as unlocked. Set rtnl_held variable that
> tracks rtnl mutex state to be false by default.
>
> Introduce tcf_proto_is_unlocked() helper that is used to check
> tcf_proto_ops->flag to determine if ops can be called without taking rtnl
> lock. Manually lookup Qdisc, class and block in rule update handlers.
> Verify that both Qdisc ops and proto ops are unlocked before using any of
> their callbacks, and obtain rtnl lock otherwise.
So if you end goal is to completely get rid of RTNL from tc filter and
action control path, why this change is needed?
I was expecting you to break down the RTNL lock down to each
block/filter chain, which should be done in this patchset. So again,
why do we still need RTNL for some cases?
Please state the reasoning in your changelog, rather than just
describing what your code does.
Thanks.
^ permalink raw reply
* Yet another approach for implementing connection tracking offload
From: Yossi Kuperman @ 2019-02-18 19:00 UTC (permalink / raw)
To: Guy Shattah, Aaron Conole, John Hurley, Simon Horman,
Justin Pettit, Gregory Rose, Eelco Chaudron, Flavio Leitner,
Florian Westphal, Jiri Pirko, Rashid Khan, Sushil Kulkarni,
Andy Gospodarek, Roi Dayan, Yossi Kuperman, Or Gerlitz,
Rony Efraim, davem@davemloft.net, Marcelo Leitner, Paul Blakey
Cc: netdev@vger.kernel.org
Hello All,
Following is a description of yet another possible approach to implement connection tracking offload. We would like to hear your opinion. There is the “native” way of implementing such an offload by mirroring the software tables to hardware. This way seems straightforward and simple, but real life is much more complicated than that. Alternatively, we can merge the data-path flows (separated by recirc_id) and offload a single flow to hardware.
The general idea is quite simple. When OVS-daemon configures TC with a filter that recirculate, the driver merely pretends to offload it and return success. Upon packet arrival (in software) we let it traverse TC as usual, except for now we notify the driver on each successful match. By doing this, the driver has all the necessary information to merge the participating flows---including connection tracking 5-tuple---into one equivalent flow. We do such a merge and offload only if the connection is established. Note: the same mechanism to communicate a 5-tuple to the driver can be used to notify on a filter match.
It is the driver responsibility to build and maintain the list of filters a (specific) packet hit along the TC walk. Once we reach the last filter (a terminating one, e.g., forward) the driver posts a work on a dedicated work-queue. In this work-queue context, we merge the participating filters and create a new filter that is logically equal (match + actions). The merge itself is not complicated as it might seems—TC does all the heavy lifting, this is not a random list of filters. At this point, we configure the hardware with one filter, either we have a match and the packet is handled by the hardware, or we don’t and the packet goes to software unmodified.
Going along this path we must tackle two things: 1) counters and 2) TC filter deletion. 1) We must maintain TC counters as the user expect. Each merged filter holds a list of filters it is derived from, parents. Once an update is available for a merged filter counter, the driver must update the corresponding parents appropriately. 2) Upon TC filer deletion it is mandatory to remove all the derived (merged) filters from the hardware as consequence.
Pros & Cons
Pros: 1) Circumvent the complexity involved with continuation in software where the hardware left off. 2) Simplifies the hardware pipeline with only one filter and might improve the overall performance.
Cons: 1) Only applicable to OVS-oriented filters, will not support priorities and overlapping filters. 2) Merger logic might consume CPU cycles which might impact the rate of filters we can offload. However, this overhead is believed to be negligible, if implemented carefully. 3) Requires TC/flower to notify the driver on each filter match (that is the only change needed above the driver).
Both approaches share the same software model, most of the code above the driver is shared. This approach can be considered temporary until the hardware will mature.
What do you think about this approach?
If something is not clear please let me know and I will do my best to clarify.
Cheers,
Kuperman
^ permalink raw reply
* Re: [PATCH 1/2] wireless: mt76: call hweight8() instead of __sw_hweight8()
From: Kalle Valo @ 2019-02-18 19:06 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Ingo Molnar, Thomas Gleixner, Borislav Petkov, H . Peter Anvin,
x86, linux-wireless, Christoph Hellwig, David S. Miller, netdev,
linux-kernel, Lorenzo Bianconi, linux-mediatek, linux-arm-kernel,
Felix Fietkau, Matthias Brugger
In-Reply-To: <877edxt1f1.fsf@kamboji.qca.qualcomm.com>
Kalle Valo <kvalo@codeaurora.org> writes:
> Masahiro Yamada <yamada.masahiro@socionext.com> writes:
>
>> __sw_hweight8() is just internal implementation.
>>
>> Drivers should use the common API, hweight8().
>>
>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>
> Acked-by: Kalle Valo <kvalo@codeaurora.org>
>
>> This patch should go to x86 tree along with 2/2.
>>
>> Otherwise, all{yes,mod}config of x86 would be broken.
>>
>> This patch is trivial enough.
>> I want ACK from the net/wireless maintainer
>> so that this can go in via x86 tree.
>
> Sounds good to me, feel free to push via the x86 tree.
Actually, can you wait a bit? Felix just reminded me that we have a
similar patch pending from Ben Hutchings:
https://patchwork.kernel.org/patch/10808203/
And it seems there are two instances of __sw_hweight8, at least in my
wireless-drivers-next tree:
drivers/net/wireless/mediatek/mt76/mac80211.c: int i, nstream = __sw_hweight8(dev->antenna_mask);
drivers/net/wireless/mediatek/mt76/mac80211.c: int n_chains = __sw_hweight8(dev->antenna_mask);
So we need to think what to do.
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH net-next 01/12] net: sched: flower: don't check for rtnl on head dereference
From: Cong Wang @ 2019-02-18 19:08 UTC (permalink / raw)
To: Vlad Buslov
Cc: Linux Kernel Network Developers, Jamal Hadi Salim, Jiri Pirko,
David Miller
In-Reply-To: <20190214074712.17846-2-vladbu@mellanox.com>
On Wed, Feb 13, 2019 at 11:47 PM Vlad Buslov <vladbu@mellanox.com> wrote:
>
> Flower classifier only changes root pointer during init and destroy. Cls
> API implements reference counting for tcf_proto, so there is no danger of
> concurrent access to tp when it is being destroyed, even without protection
> provided by rtnl lock.
How about atomicity? Refcnt doesn't guarantee atomicity, how do
you make sure two concurrent modifications are atomic?
>
> Implement new function fl_head_dereference() to dereference tp->root
> without checking for rtnl lock. Use it in all flower function that obtain
> head pointer instead of rtnl_dereference().
>
So what lock protects RCU writers after this patch?
^ permalink raw reply
* Re: [BUG] [FIX] net: dsa: oops in br_vlan_enabled
From: Florian Fainelli @ 2019-02-18 19:09 UTC (permalink / raw)
To: Frank Wunderlich; +Cc: netdev, Andrew Lunn, Vivien Didelot
In-Reply-To: <trinity-d285d34c-7cf1-47bc-a86e-ab4e59a194b1-1550509887417@3c-app-gmx-bs58>
On February 18, 2019 9:11:27 AM PST, Frank Wunderlich <frank-w@public-files.de> wrote:
>Hi,
>
>tried your Patch but crashes the same way:
Yes the lower_dev targeted by switched does point to the DSA slave network device, but we really don't have dp->bridge_dev assigned since the physical DSA port was not enslaved in the bridge. Will follow up with a correct version late today.
>
>[ 107.416972] [<bf0fde88>] (br_vlan_enabled [bridge]) from
>[<bf14e234>] (dsa_po
>rt_vlan_add+0x60/0xbc [dsa_core])
>
>[ 107.426939] [<bf14e234>] (dsa_port_vlan_add [dsa_core]) from
>[<bf14eba4>] (ds
>a_slave_port_obj_add+0x64/0x68 [dsa_core])
>
>[ 107.437666] [<bf14eba4>] (dsa_slave_port_obj_add [dsa_core]) from
>[<c0b4e684>
>] (__switchdev_port_obj_add+0x50/0xc4)
>
>[ 107.448029] [<c0b4e684>] (__switchdev_port_obj_add) from
>[<c0b4e6d4>] (__swit
>chdev_port_obj_add+0xa0/0xc4)
>
>[ 107.457612] [<c0b4e6d4>] (__switchdev_port_obj_add) from
>[<c0b4e758>] (switch
>dev_port_obj_add_now+0x60/0x130)
>
>[ 107.467453] [<c0b4e758>] (switchdev_port_obj_add_now) from
>[<c0b4eb94>] (swit
>chdev_port_obj_add+0x44/0x190)
>
>[ 107.477181] [<c0b4eb94>] (switchdev_port_obj_add) from [<bf1013d0>]
>(br_switc
>hdev_port_vlan_add+0x60/0x7c [bridge])
>
>[ 107.487644] [<bf1013d0>] (br_switchdev_port_vlan_add [bridge]) from
>[<bf0ff25
>0>] (__vlan_add+0xb0/0x620 [bridge])
>
>regards Frank
>
>
>> Gesendet: Montag, 18. Februar 2019 um 00:20 Uhr
>> Von: "Florian Fainelli" <f.fainelli@gmail.com>
>> An: "Frank Wunderlich" <frank-w@public-files.de>,
>netdev@vger.kernel.org, "Andrew Lunn" <andrew@lunn.ch>, "Vivien
>Didelot" <vivien.didelot@gmail.com>
>> Betreff: Re: [BUG] [FIX] net: dsa: oops in br_vlan_enabled
>>
>>
>>
>> On 2/17/2019 9:13 AM, Florian Fainelli wrote:
>> > Hi Frank,
>> >
>> > On 2/16/2019 8:22 AM, Frank Wunderlich wrote:
>> >> Hi,
>> >>
>> >> i've found an oops in 4.19.23/10, seems to be fixed anyhow in 5.0
>(also works in 4.14.101)
>> >>
>> >> root@bpi-r2:~# ip link add link lan0 name lan0.5 type vlan id 5
>> >> root@bpi-r2:~# ip addr add 192.168.5.200/24 brd 192.168.5.255 dev
>lan0.5
>> >> root@bpi-r2:~# ip link set dev lan0 up
>> >> root@bpi-r2:~# ip link set dev lan0.5 up
>> >
>> > So that these steps don't involve a bridge, and because we don't
>(yet)
>> > implment ndo_vlan_rx_{add,kill}_vid() netdevice_ops, there is no
>VLAN
>> > programming, it's all software
>> >
>> >>
>> >> 12: lan0.5@lan0: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500
>qdisc noqueue state LOWERLAYERDOWN group default qlen 1000
>> >> link/ether 02:02:02:02:02:02 brd ff:ff:ff:ff:ff:ff
>> >> inet 192.168.5.200/24 brd 192.168.5.255 scope global lan0.5
>> >> valid_lft forever preferred_lft forever
>> >>
>> >> root@bpi-r2:~# brctl addbr bridge_name
>> >> root@bpi-r2:~# brctl addif bridge_name lan0.5
>> >
>> > Unless you changed the bridge to have VLAN filtering/awareness
>with:
>> >
>> > echo 1 > /sys/class/net/bridge_name/bridge/vlan_filtering
>> >
>> > There will be no VLAN configuration/objects pushed to DSA since
>commit
>> > 2ea7a679ca2abd251c1ec03f20508619707e1749 ("net: dsa: Don't add
>vlans
>> > when vlan filtering is disabled") so I am not sure how you got into
>that
>> > situation because prior to pushing a VLAN object, the port must be
>part
>> > of a bridge, so the steps typically look like:
>> >
>> > - port_bridge_join which assigns dp->bridge_dev
>> > - port_vlan_add
>> >
>> > Does your 4.19.23 kernel somehow change how VLAN objects are pushed
>down
>> > the switch driver?
>> >
>> >> [ 352.057128] bridge_name: port 1(lan0.5) entered blocking state
>> >> [ 352.063065] bridge_name: port 1(lan0.5) entered disabled state
>> >> [ 352.069181] device lan0.5 entered promiscuous mode
>> >> [ 352.074018] device lan0 entered promiscuous mode
>> >> [ 352.078906] Unable to handle kernel NULL pointer dereference at
>virtual address 00000558
>> >> ...
>> >> [ 352.493085] [<bf0fde88>] (br_vlan_enabled [bridge]) from
>[<bf12c234>] (dsa_port_vlan_add+0x60/0xbc [dsa_core])
>> >> [ 352.503050] [<bf12c234>] (dsa_port_vlan_add [dsa_core]) from
>[<bf12cb64>] (dsa_slave_port_obj_add+0x4c/0x50 [dsa_core])
>> >> [ 352.513776] [<bf12cb64>] (dsa_slave_port_obj_add [dsa_core])
>from [<c0b4e2d4>] (__switchdev_port_obj_add+0x50/0xc4)
>> >> [ 352.524138] [<c0b4e2d4>] (__switchdev_port_obj_add) from
>[<c0b4e324>] (__switchdev_port_obj_add+0xa0/0xc4)
>> >> [ 352.533721] [<c0b4e324>] (__switchdev_port_obj_add) from
>[<c0b4e3a8>] (switchdev_port_obj_add_now+0x60/0x130)
>> >> [ 352.543562] [<c0b4e3a8>] (switchdev_port_obj_add_now) from
>[<c0b4e7e4>] (switchdev_port_obj_add+0x44/0x190)
>> >> [ 352.553284] [<c0b4e7e4>] (switchdev_port_obj_add) from
>[<bf1013d0>] (br_switchdev_port_vlan_add+0x60/0x7c [bridge])
>> >> [ 352.563733] [<bf1013d0>] (br_switchdev_port_vlan_add [bridge])
>from [<bf0ff250>] (__vlan_add+0xb0/0x620 [bridge])
>> >> [ 352.574007] [<bf0ff250>] (__vlan_add [bridge]) from
>[<bf0ffd04>] (nbp_vlan_add+0xc4/0x150 [bridge])
>> >> [ 352.583073] [<bf0ffd04>] (nbp_vlan_add [bridge]) from
>[<bf0ffec4>] (nbp_vlan_init+0x134/0x164 [bridge])
>> >> [ 352.592482] [<bf0ffec4>] (nbp_vlan_init [bridge]) from
>[<bf0edd4c>] (br_add_if+0x40c/0x5fc [bridge])
>> >> [ 352.601632] [<bf0edd4c>] (br_add_if [bridge]) from [<bf0eeb14>]
>(add_del_if+0x6c/0x80 [bridge])
>> >> [ 352.610351] [<bf0eeb14>] (add_del_if [bridge]) from
>[<bf0ef5b0>] (br_dev_ioctl+0x7c/0x9c [bridge])
>> >> [ 352.619290] [<bf0ef5b0>] (br_dev_ioctl [bridge]) from
>[<c09583d4>] (dev_ifsioc+0x184/0x324)
>> >> [ 352.627582] [<c09583d4>] (dev_ifsioc) from [<c09589e8>]
>(dev_ioctl+0x32c/0x5cc)
>> >> [ 352.634837] [<c09589e8>] (dev_ioctl) from [<c090913c>]
>(sock_ioctl+0x3bc/0x580)
>> >>
>> >>
>> >> since my 4.19.23 kernel is modified a bit i tried with 4.19.10
>without my net modifications and it is still reproducable with steps
>above (create a vlan on dsa-user-port and then use it in a bridge)
>> >>
>> >> i fixed it with these changes:
>>
>> Your fix is undoing the commit from Andrew I just referenced, so it
>is
>> definitively not the right fix because it will push VLAN objects down
>to
>> a non-VLAN aware bridge, not that this is really a problem, but it
>> should not be happening anyway.
>>
>> The problem appears to be the following though: you are enslaving a
>VLAN
>> device, which does not have switchdev_ops, so we recurse into the
>lower
>> device, which is the DSA network device, which does have
>switchdev_ops
>> defined. Once we are there we check the orig_dev against being a
>bridge
>> master network device, but we are not checking that it's a VLAN
>device
>> so we end-up assuming it is a DSA network device, we de-reference
>> garbage by checking br_vlan_enabled(dp->bridge_dev) because there is
>> simply no such data structure associated with the VLAN device.
>>
>> The attached patch should help.
>>
>> This is no longer a problem in newer kernels because the switchdev
>> operations use a notifier which checks the target network device to
>be DSA.
>> --
>> Florian
>>
--
Florian
^ permalink raw reply
* Re: [PATCH net] net: validate untrusted gso packets without csum offload
From: Willem de Bruijn @ 2019-02-18 19:12 UTC (permalink / raw)
To: Network Development
Cc: David Miller, Eric Dumazet, Jason Wang, Maxim Mikityanskiy,
Willem de Bruijn, syzbot
In-Reply-To: <20190215171547.247018-1-willemdebruijn.kernel@gmail.com>
On Fri, Feb 15, 2019 at 12:15 PM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> From: Willem de Bruijn <willemb@google.com>
>
> Syzkaller again found a path to a kernel crash through bad gso input.
> By building an excessively large packet to cause an skb field to wrap.
>
> If VIRTIO_NET_HDR_F_NEEDS_CSUM was set this would have been dropped in
> skb_partial_csum_set.
>
> GSO packets that do not set checksum offload are suspicious and rare.
> Most callers of virtio_net_hdr_to_skb already pass them to
> skb_probe_transport_header.
>
> Move that test forward, change it to detect parse failure and drop
> packets on failure as those cleary are not one of the legitimate
> VIRTIO_NET_HDR_GSO types.
>
> Fixes: bfd5f4a3d605 ("packet: Add GSO/csum offload support.")
> Fixes: f43798c27684 ("tun: Allow GSO using virtio_net_hdr")
> Reported-by: syzbot <syzkaller@googlegroups.com>
> Signed-off-by: Willem de Bruijn <willemb@google.com>
This causes false positive drops on virtio-net and tun for these
packets with gso without csum_off. And on pf_packet with proto 0.
This happens because skb->protocol is set in these callers after
the call to virtio_net_hdr_to_skb. And the flow dissector relies on
this to start dissection, not the link layer header (if present).
Moving this logic forward is too much churn for net, especially since
eth_type_header pulls the header, requiring additional changes to
adjust csum_start.
virtio_net_hdr_set_proto() aims to fix this by deriving skb->protocol
from the gso_type. But unfortunately for UDP it unconditionally
selects ipv4, which will cause drops for UDP over ipv6.
For net I plan to just ignore the error for these callers that do not
set skb->protocol.
- if (!skb_transport_header_was_set(skb))
+ if (!skb_transport_header_was_set(skb) && skb->protocol)
Possibly with an extension of tpacket_set_protocol to also cover
packet_snd, so that that cannot evade it on purpose. Other callers can
wait till net-next.
>
> ---
>
> This captures a variety of bad gso packets, but to tighten further:
>
> - drop SKB_GSO_DODGY packets with ipip/sit/.. , which cannot be legal.
> by ipip_gso_segment wrappers around inet_gso_segment
> expands on 121d57af308d ("gso: validate gso_type in GSO handlers")
>
> - limit the number of ipv6 exthdrs allowed from dodgy sources.
> not sure where to draw the line. but not at 64K ;)
This already exists, in the form of skb_flow_dissect_allowed
> - validate the network and transport protocol returned in
> skb_probe_transport_header against the VIRTIO_NET_HDR_GSO type
>
> - probe all dodgy GSO packets, also those that set checksum offload.
> this will have a performance impact, discussed previously in
> http://patchwork.ozlabs.org/patch/861874/
> but it would have blocked this latest bug as well
>
> All but the last one seem pretty uncontroversial to me. If no one
> objects I plan to send those to net-next.
>
> ---
> include/linux/skbuff.h | 2 +-
> include/linux/virtio_net.h | 9 +++++++++
> 2 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 95d25b010a25..4c1c82a5678c 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -2434,7 +2434,7 @@ static inline void skb_probe_transport_header(struct sk_buff *skb,
>
> if (skb_flow_dissect_flow_keys_basic(skb, &keys, NULL, 0, 0, 0, 0))
> skb_set_transport_header(skb, keys.control.thoff);
> - else
> + else if (offset_hint >= 0)
> skb_set_transport_header(skb, offset_hint);
> }
>
> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> index cb462f9ab7dd..71f2394abbf7 100644
> --- a/include/linux/virtio_net.h
> +++ b/include/linux/virtio_net.h
> @@ -57,6 +57,15 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
>
> if (!skb_partial_csum_set(skb, start, off))
> return -EINVAL;
> + } else {
> + /* gso packets without NEEDS_CSUM do not set transport_offset.
> + * probe and drop if does not match one of the above types.
> + */
> + if (gso_type) {
> + skb_probe_transport_header(skb, -1);
> + if (!skb_transport_header_was_set(skb))
> + return -EINVAL;
> + }
> }
>
> if (hdr->gso_type != VIRTIO_NET_HDR_GSO_NONE) {
> --
> 2.21.0.rc0.258.g878e2cd30e-goog
>
^ permalink raw reply
* Re: [PATCH net-next 00/12] Refactor flower classifier to remove dependency on rtnl lock
From: Cong Wang @ 2019-02-18 19:15 UTC (permalink / raw)
To: Vlad Buslov
Cc: Linux Kernel Network Developers, Jamal Hadi Salim, Jiri Pirko,
David Miller
In-Reply-To: <20190214074712.17846-1-vladbu@mellanox.com>
Hi,
> net/sched/cls_flower.c | 424 +++++++++++++++++++++++++++++++++++++------------
> 1 file changed, 321 insertions(+), 103 deletions(-)
>
Given you change cls_flower so much, please also add a test case for
verifying your changes, especially focusing on the atomicity of concurrent
modifications.
Thanks.
^ permalink raw reply
* Re: [PATCH 1/2] wireless: mt76: call hweight8() instead of __sw_hweight8()
From: Kalle Valo @ 2019-02-18 19:20 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Ingo Molnar, Thomas Gleixner, Borislav Petkov, H . Peter Anvin,
x86, linux-wireless, Christoph Hellwig, David S. Miller, netdev,
linux-kernel, Lorenzo Bianconi, linux-mediatek, linux-arm-kernel,
Felix Fietkau, Matthias Brugger
In-Reply-To: <87tvh0syq2.fsf@kamboji.qca.qualcomm.com>
Kalle Valo <kvalo@codeaurora.org> writes:
> Kalle Valo <kvalo@codeaurora.org> writes:
>
>> Masahiro Yamada <yamada.masahiro@socionext.com> writes:
>>
>>> __sw_hweight8() is just internal implementation.
>>>
>>> Drivers should use the common API, hweight8().
>>>
>>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>>
>> Acked-by: Kalle Valo <kvalo@codeaurora.org>
>>
>>> This patch should go to x86 tree along with 2/2.
>>>
>>> Otherwise, all{yes,mod}config of x86 would be broken.
>>>
>>> This patch is trivial enough.
>>> I want ACK from the net/wireless maintainer
>>> so that this can go in via x86 tree.
>>
>> Sounds good to me, feel free to push via the x86 tree.
>
> Actually, can you wait a bit? Felix just reminded me that we have a
> similar patch pending from Ben Hutchings:
>
> https://patchwork.kernel.org/patch/10808203/
>
> And it seems there are two instances of __sw_hweight8, at least in my
> wireless-drivers-next tree:
>
> drivers/net/wireless/mediatek/mt76/mac80211.c: int i, nstream =
> __sw_hweight8(dev->antenna_mask);
> drivers/net/wireless/mediatek/mt76/mac80211.c: int n_chains =
> __sw_hweight8(dev->antenna_mask);
>
> So we need to think what to do.
I think the best would be that Felix applies Ben's patch to the wireless
tree and I'll push it to Linus during the next merge window (via
net-next). Simpler and less conflicts that way.
So Masahiro, could you hold your x86 patch for a while until the mt76
patch will be in Linus' tree? That should not take more than two weeks,
I think.
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH net-next 12/12] net: sched: flower: set unlocked flag for flower proto ops
From: Cong Wang @ 2019-02-18 19:27 UTC (permalink / raw)
To: Vlad Buslov
Cc: Linux Kernel Network Developers, Jamal Hadi Salim, Jiri Pirko,
David Miller
In-Reply-To: <20190214074712.17846-13-vladbu@mellanox.com>
On Wed, Feb 13, 2019 at 11:47 PM Vlad Buslov <vladbu@mellanox.com> wrote:
>
> Set TCF_PROTO_OPS_DOIT_UNLOCKED for flower classifier to indicate that its
> ops callbacks don't require caller to hold rtnl lock.
So, if this means RTNL is gone for all cls_flower changes, why
do I still see rtnl_lock() in cls_flower.c after all your patches in this set?
For instance:
366 static void fl_destroy_filter_work(struct work_struct *work)
367 {
368 struct cls_fl_filter *f = container_of(to_rcu_work(work),
369 struct cls_fl_filter, rwork);
370
371 rtnl_lock();
372 __fl_destroy_filter(f);
373 rtnl_unlock();
374 }
and...
382 if (!rtnl_held)
383 rtnl_lock();
...
1436 if (!rtnl_held)
1437 rtnl_lock();
Please explain in your changelog, otherwise it is very confusing.
Thanks.
^ permalink raw reply
* [PATCH net-next 0/2] net: phy: improve genphy_read_status
From: Heiner Kallweit @ 2019-02-18 19:28 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev@vger.kernel.org
Series includes two smaller improvements to genphy_read_status.
Heiner Kallweit (2):
net: phy: remove orphaned register read in genphy_read_status
net: phy: don't use 10BaseT/half as default in genphy_read_status
drivers/net/phy/phy_device.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
--
2.20.1
^ permalink raw reply
* [PATCH net-next 1/2] net: phy: remove orphaned register read in genphy_read_status
From: Heiner Kallweit @ 2019-02-18 19:28 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev@vger.kernel.org
In-Reply-To: <1a948543-0dc6-d5df-d50a-49134a2a4118@gmail.com>
After recent changes to genphy_read_status() this orphaned register
read remained as leftover. So remove it.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/phy/phy_device.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 7e71124bb..803197fdd 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1785,10 +1785,6 @@ int genphy_read_status(struct phy_device *phydev)
mii_lpa_mod_linkmode_lpa_t(phydev->lp_advertising, lpa);
- adv = phy_read(phydev, MII_ADVERTISE);
- if (adv < 0)
- return adv;
-
phydev->speed = SPEED_10;
phydev->duplex = DUPLEX_HALF;
phydev->pause = 0;
--
2.20.1
^ permalink raw reply related
* [PATCH net-next 2/2] net: phy: don't use 10BaseT/half as default in genphy_read_status
From: Heiner Kallweit @ 2019-02-18 19:29 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev@vger.kernel.org
In-Reply-To: <1a948543-0dc6-d5df-d50a-49134a2a4118@gmail.com>
If link partner and we can't agree on any mode, then it doesn't make
sense to pretend we would have agreed on 10/half. Therefore set a
proper default.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/phy/phy_device.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 803197fdd..4bb3b6c28 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1785,8 +1785,8 @@ int genphy_read_status(struct phy_device *phydev)
mii_lpa_mod_linkmode_lpa_t(phydev->lp_advertising, lpa);
- phydev->speed = SPEED_10;
- phydev->duplex = DUPLEX_HALF;
+ phydev->speed = SPEED_UNKNOWN;
+ phydev->duplex = DUPLEX_UNKNOWN;
phydev->pause = 0;
phydev->asym_pause = 0;
--
2.20.1
^ permalink raw reply related
* Re: TC stats / hw offload question
From: Edward Cree @ 2019-02-18 19:37 UTC (permalink / raw)
To: Jamal Hadi Salim, netdev
Cc: Jiri Pirko, Cong Wang, Or Gerlitz, Andy Gospodarek, PJ Waskiewicz,
Anjali Singhai Jain, Jakub Kicinski
In-Reply-To: <5930f8e7-3615-5470-ea09-f0e6f2a3a3d7@solarflare.com>
On 18/02/19 18:56, Edward Cree wrote:
> (or
> maybe require it to be set to some distinguished value, like 0, to
> mean 'allocate', so that as a future extension we can support
> shareable counters).
Turns out 0 isn't an option, because TC already internally swallows
that and allocates its own index (same as if 'index' wasn't specified
on cmdline at all), unless we can change TC to pass that 0 on to the
driver.
-Ed
^ permalink raw reply
* Re: [PATCH net-next v4 10/17] net: sched: refactor tp insert/delete for concurrent execution
From: Cong Wang @ 2019-02-18 19:53 UTC (permalink / raw)
To: Vlad Buslov
Cc: Linux Kernel Network Developers, Jamal Hadi Salim, Jiri Pirko,
David Miller, Alexei Starovoitov, Daniel Borkmann
In-Reply-To: <20190211085548.7190-11-vladbu@mellanox.com>
On Mon, Feb 11, 2019 at 12:56 AM Vlad Buslov <vladbu@mellanox.com> wrote:
> +#define tcf_proto_dereference(p, tp) \
> + rcu_dereference_protected(p, lockdep_tcf_proto_is_locked(tp))
> +
BTW, it is not used anywhere even on top of your cls_flower changes...
$ git grep tcf_proto_dereference
include/net/sch_generic.h:#define tcf_proto_dereference(p, tp)
\
Please don't introduce new things unitl you actually use it.
^ permalink raw reply
* Re: [PATCH net-next v4 10/17] net: sched: refactor tp insert/delete for concurrent execution
From: Cong Wang @ 2019-02-18 19:55 UTC (permalink / raw)
To: Vlad Buslov
Cc: Linux Kernel Network Developers, Jamal Hadi Salim, Jiri Pirko,
David Miller, Alexei Starovoitov, Daniel Borkmann
In-Reply-To: <vbfimxhqr7z.fsf@mellanox.com>
On Mon, Feb 18, 2019 at 3:19 AM Vlad Buslov <vladbu@mellanox.com> wrote:
>
>
> On Fri 15 Feb 2019 at 23:17, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> > On Mon, Feb 11, 2019 at 12:56 AM Vlad Buslov <vladbu@mellanox.com> wrote:
> >> +static bool tcf_proto_is_empty(struct tcf_proto *tp)
> >> +{
> >> + struct tcf_walker walker = { .fn = walker_noop, };
> >> +
> >> + if (tp->ops->walk) {
> >> + tp->ops->walk(tp, &walker);
> >> + return !walker.stop;
> >> + }
> >> + return true;
> >> +}
> >> +
> >> +static bool tcf_proto_check_delete(struct tcf_proto *tp)
> >> +{
> >> + spin_lock(&tp->lock);
> >> + if (tcf_proto_is_empty(tp))
> >> + tp->deleting = true;
> >> + spin_unlock(&tp->lock);
> >> + return tp->deleting;
> >
> > If you use this spinlock for walking each tp data structure,
> > why it is not needed for adding to/deleting filters from each
> > tp?
>
> This lock is intended to be used by unlocked classifiers and I use it in
> my following flower patch set extensively. Classifiers that do not set
> 'unlocked' flag continue to rely on rtnl lock for synchronization.
It is never late to add it when you seriously use it. The way you
split the patches is really annoying for reviewers...
^ permalink raw reply
* Re: [PATCH 00/11] Netfilter/IPVS updates for net-next
From: David Miller @ 2019-02-18 19:59 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel, netdev
In-Reply-To: <20190217232357.29858-1-pablo@netfilter.org>
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Mon, 18 Feb 2019 00:23:46 +0100
> The following patchset contains Netfilter/IPVS updates for you net-next
> tree:
...
> You can pull these changes from:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf-next.git
Pulled, thanks Pablo.
^ permalink raw reply
* Re: [PATCH net 0/2] tcp: fix possible crash in tcp_v4_err()
From: David Miller @ 2019-02-18 19:59 UTC (permalink / raw)
To: eric.dumazet; +Cc: edumazet, netdev, ncardwell, ycheng, soukjin.bae
In-Reply-To: <8e6280f3-ea5b-48bd-4dc0-da134d4125d2@gmail.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Sun, 17 Feb 2019 18:20:21 -0800
>
>
> On 02/17/2019 03:47 PM, David Miller wrote:
>> From: Eric Dumazet <edumazet@google.com>
>> Date: Fri, 15 Feb 2019 13:36:19 -0800
>>
>>> soukjin bae reported a crash in tcp_v4_err() that we
>>> root caused to a missing initialization.
>>>
>>> Second patch adds a sanity check in tcp_v4_err() to avoid
>>> future potential problems. Ignoring an ICMP message
>>> is probably better than crashing a machine.
>>
>> Series applied, thanks Eric.
>>
>> Want me to queue these up for -stable?
>>
>
> Yes please, I put no Fixes: tag because it seemed to be a day-0 bug.
Done.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox