From 40171248bb8934537fec8fbaf718e57c8add187c Mon Sep 17 00:00:00 2001 From: Xin Long Date: Thu, 28 Oct 2021 05:36:01 -0400 Subject: [PATCH 01/17] sctp: allow IP fragmentation when PLPMTUD enters Error state Currently when PLPMTUD enters Error state, transport pathmtu will be set to MIN_PLPMTU(512) while probe is continuing with BASE_PLPMTU(1200). It will cause pathmtu to stay in a very small value, even if the real pmtu is some value like 1000. RFC8899 doesn't clearly say how to set the value in Error state. But one possibility could be keep using BASE_PLPMTU for the real pmtu, but allow to do IP fragmentation when it's in Error state. As it says in rfc8899#section-5.4: Some paths could be unable to sustain packets of the BASE_PLPMTU size. The Error State could be implemented to provide robustness to such paths. This allows fallback to a smaller than desired PLPMTU rather than suffer connectivity failure. This could utilize methods such as endpoint IP fragmentation to enable the PL sender to communicate using packets smaller than the BASE_PLPMTU. This patch is to set pmtu to BASE_PLPMTU instead of MIN_PLPMTU for Error state in sctp_transport_pl_send/toobig(), and set packet ipfragok for non-probe packets when it's in Error state. Fixes: 1dc68c194571 ("sctp: do state transition when PROBE_COUNT == MAX_PROBES on HB send path") Reported-by: Ying Xu Signed-off-by: Xin Long Signed-off-by: David S. Miller --- net/sctp/output.c | 13 ++++++++----- net/sctp/transport.c | 4 ++-- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/net/sctp/output.c b/net/sctp/output.c index 4dfb5ea82b05..cdfdbd353c67 100644 --- a/net/sctp/output.c +++ b/net/sctp/output.c @@ -581,13 +581,16 @@ int sctp_packet_transmit(struct sctp_packet *packet, gfp_t gfp) chunk = list_entry(packet->chunk_list.next, struct sctp_chunk, list); sk = chunk->skb->sk; - /* check gso */ if (packet->size > tp->pathmtu && !packet->ipfragok && !chunk->pmtu_probe) { - if (!sk_can_gso(sk)) { - pr_err_once("Trying to GSO but underlying device doesn't support it."); - goto out; + if (tp->pl.state == SCTP_PL_ERROR) { /* do IP fragmentation if in Error state */ + packet->ipfragok = 1; + } else { + if (!sk_can_gso(sk)) { /* check gso */ + pr_err_once("Trying to GSO but underlying device doesn't support it."); + goto out; + } + gso = 1; } - gso = 1; } /* alloc head skb */ diff --git a/net/sctp/transport.c b/net/sctp/transport.c index a3d3ca6dd63d..1f2dfad768d5 100644 --- a/net/sctp/transport.c +++ b/net/sctp/transport.c @@ -269,7 +269,7 @@ bool sctp_transport_pl_send(struct sctp_transport *t) if (t->pl.probe_size == SCTP_BASE_PLPMTU) { /* BASE_PLPMTU Confirmation Failed */ t->pl.state = SCTP_PL_ERROR; /* Base -> Error */ - t->pl.pmtu = SCTP_MIN_PLPMTU; + t->pl.pmtu = SCTP_BASE_PLPMTU; t->pathmtu = t->pl.pmtu + sctp_transport_pl_hlen(t); sctp_assoc_sync_pmtu(t->asoc); } @@ -366,7 +366,7 @@ static bool sctp_transport_pl_toobig(struct sctp_transport *t, u32 pmtu) if (pmtu >= SCTP_MIN_PLPMTU && pmtu < SCTP_BASE_PLPMTU) { t->pl.state = SCTP_PL_ERROR; /* Base -> Error */ - t->pl.pmtu = SCTP_MIN_PLPMTU; + t->pl.pmtu = SCTP_BASE_PLPMTU; t->pathmtu = t->pl.pmtu + sctp_transport_pl_hlen(t); } } else if (t->pl.state == SCTP_PL_SEARCH) { From c6ea04ea692fa0d8e7faeb133fcd28e3acf470a0 Mon Sep 17 00:00:00 2001 From: Xin Long Date: Thu, 28 Oct 2021 05:36:02 -0400 Subject: [PATCH 02/17] sctp: reset probe_timer in sctp_transport_pl_update sctp_transport_pl_update() is called when transport update its dst and pathmtu, instead of stopping the PLPMTUD probe timer, PLPMTUD should start over and reset the probe timer. Otherwise, the PLPMTUD service would stop. Fixes: 92548ec2f1f9 ("sctp: add the probe timer in transport for PLPMTUD") Signed-off-by: Xin Long Signed-off-by: David S. Miller --- include/net/sctp/sctp.h | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h index 69bab88ad66b..bc00410223b0 100644 --- a/include/net/sctp/sctp.h +++ b/include/net/sctp/sctp.h @@ -653,12 +653,10 @@ static inline void sctp_transport_pl_update(struct sctp_transport *t) if (t->pl.state == SCTP_PL_DISABLED) return; - if (del_timer(&t->probe_timer)) - sctp_transport_put(t); - t->pl.state = SCTP_PL_BASE; t->pl.pmtu = SCTP_BASE_PLPMTU; t->pl.probe_size = SCTP_BASE_PLPMTU; + sctp_transport_reset_probe_timer(t); } static inline bool sctp_transport_pl_enabled(struct sctp_transport *t) From cc4665ca646c96181a7c00198aa72c59e0c576e8 Mon Sep 17 00:00:00 2001 From: Xin Long Date: Thu, 28 Oct 2021 05:36:03 -0400 Subject: [PATCH 03/17] sctp: subtract sctphdr len in sctp_transport_pl_hlen sctp_transport_pl_hlen() is called to calculate the outer header length for PL. However, as the Figure in rfc8899#section-4.4: Any additional headers .--- MPS -----. | | | v v v +------------------------------+ | IP | ** | PL | protocol data | +------------------------------+ <----- PLPMTU -----> <---------- PMTU --------------> Outer header are IP + Any additional headers, which doesn't include Packetization Layer itself header, namely sctphdr, whereas sctphdr is counted by __sctp_mtu_payload(). The incorrect calculation caused the link pathmtu to be set larger than expected by t->pl.pmtu + sctp_transport_pl_hlen(). This patch is to fix it by subtracting sctphdr len in sctp_transport_pl_hlen(). Fixes: d9e2e410ae30 ("sctp: add the constants/variables and states and some APIs for transport") Signed-off-by: Xin Long Signed-off-by: David S. Miller --- include/net/sctp/sctp.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h index bc00410223b0..189fdb9db162 100644 --- a/include/net/sctp/sctp.h +++ b/include/net/sctp/sctp.h @@ -626,7 +626,8 @@ static inline __u32 sctp_min_frag_point(struct sctp_sock *sp, __u16 datasize) static inline int sctp_transport_pl_hlen(struct sctp_transport *t) { - return __sctp_mtu_payload(sctp_sk(t->asoc->base.sk), t, 0, 0); + return __sctp_mtu_payload(sctp_sk(t->asoc->base.sk), t, 0, 0) - + sizeof(struct sctphdr); } static inline void sctp_transport_pl_reset(struct sctp_transport *t) From 75cf662c64dd8543f56c329c69eba18141c8fd9f Mon Sep 17 00:00:00 2001 From: Xin Long Date: Thu, 28 Oct 2021 05:36:04 -0400 Subject: [PATCH 04/17] sctp: return true only for pathmtu update in sctp_transport_pl_toobig sctp_transport_pl_toobig() supposes to return true only if there's pathmtu update, so that in sctp_icmp_frag_needed() it would call sctp_assoc_sync_pmtu() and sctp_retransmit(). This patch is to fix these return places in sctp_transport_pl_toobig(). Fixes: 836964083177 ("sctp: do state transition when receiving an icmp TOOBIG packet") Signed-off-by: Xin Long Signed-off-by: David S. Miller --- net/sctp/transport.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/net/sctp/transport.c b/net/sctp/transport.c index 1f2dfad768d5..133f1719bf1b 100644 --- a/net/sctp/transport.c +++ b/net/sctp/transport.c @@ -368,6 +368,7 @@ static bool sctp_transport_pl_toobig(struct sctp_transport *t, u32 pmtu) t->pl.pmtu = SCTP_BASE_PLPMTU; t->pathmtu = t->pl.pmtu + sctp_transport_pl_hlen(t); + return true; } } else if (t->pl.state == SCTP_PL_SEARCH) { if (pmtu >= SCTP_BASE_PLPMTU && pmtu < t->pl.pmtu) { @@ -378,11 +379,10 @@ static bool sctp_transport_pl_toobig(struct sctp_transport *t, u32 pmtu) t->pl.probe_high = 0; t->pl.pmtu = SCTP_BASE_PLPMTU; t->pathmtu = t->pl.pmtu + sctp_transport_pl_hlen(t); + return true; } else if (pmtu > t->pl.pmtu && pmtu < t->pl.probe_size) { t->pl.probe_size = pmtu; t->pl.probe_count = 0; - - return false; } } else if (t->pl.state == SCTP_PL_COMPLETE) { if (pmtu >= SCTP_BASE_PLPMTU && pmtu < t->pl.pmtu) { @@ -393,10 +393,11 @@ static bool sctp_transport_pl_toobig(struct sctp_transport *t, u32 pmtu) t->pl.probe_high = 0; t->pl.pmtu = SCTP_BASE_PLPMTU; t->pathmtu = t->pl.pmtu + sctp_transport_pl_hlen(t); + return true; } } - return true; + return false; } bool sctp_transport_update_pmtu(struct sctp_transport *t, u32 pmtu) From daf182d360e509a494db18666799f4e85d83dda0 Mon Sep 17 00:00:00 2001 From: Shyam Sundar S K Date: Wed, 27 Oct 2021 15:27:27 +0530 Subject: [PATCH 05/17] net: amd-xgbe: Toggle PLL settings during rate change For each rate change command submission, the FW has to do a phy power off sequence internally. For this to happen correctly, the PLL re-initialization control setting has to be turned off before sending mailbox commands and re-enabled once the command submission is complete. Without the PLL control setting, the link up takes longer time in a fixed phy configuration. Fixes: 47f164deab22 ("amd-xgbe: Add PCI device support") Co-developed-by: Sudheesh Mavila Signed-off-by: Sudheesh Mavila Signed-off-by: Shyam Sundar S K Acked-by: Tom Lendacky Signed-off-by: David S. Miller --- drivers/net/ethernet/amd/xgbe/xgbe-common.h | 8 ++++++++ drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c | 20 +++++++++++++++++++- 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-common.h b/drivers/net/ethernet/amd/xgbe/xgbe-common.h index b2cd3bdba9f8..533b8519ec35 100644 --- a/drivers/net/ethernet/amd/xgbe/xgbe-common.h +++ b/drivers/net/ethernet/amd/xgbe/xgbe-common.h @@ -1331,6 +1331,10 @@ #define MDIO_VEND2_PMA_CDR_CONTROL 0x8056 #endif +#ifndef MDIO_VEND2_PMA_MISC_CTRL0 +#define MDIO_VEND2_PMA_MISC_CTRL0 0x8090 +#endif + #ifndef MDIO_CTRL1_SPEED1G #define MDIO_CTRL1_SPEED1G (MDIO_CTRL1_SPEED10G & ~BMCR_SPEED100) #endif @@ -1389,6 +1393,10 @@ #define XGBE_PMA_RX_RST_0_RESET_ON 0x10 #define XGBE_PMA_RX_RST_0_RESET_OFF 0x00 +#define XGBE_PMA_PLL_CTRL_MASK BIT(15) +#define XGBE_PMA_PLL_CTRL_ENABLE BIT(15) +#define XGBE_PMA_PLL_CTRL_DISABLE 0x0000 + /* Bit setting and getting macros * The get macro will extract the current bit field value from within * the variable diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c b/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c index 18e48b3bc402..213769054391 100644 --- a/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c +++ b/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c @@ -1977,12 +1977,26 @@ static void xgbe_phy_rx_reset(struct xgbe_prv_data *pdata) } } +static void xgbe_phy_pll_ctrl(struct xgbe_prv_data *pdata, bool enable) +{ + XMDIO_WRITE_BITS(pdata, MDIO_MMD_PMAPMD, MDIO_VEND2_PMA_MISC_CTRL0, + XGBE_PMA_PLL_CTRL_MASK, + enable ? XGBE_PMA_PLL_CTRL_ENABLE + : XGBE_PMA_PLL_CTRL_DISABLE); + + /* Wait for command to complete */ + usleep_range(100, 200); +} + static void xgbe_phy_perform_ratechange(struct xgbe_prv_data *pdata, unsigned int cmd, unsigned int sub_cmd) { unsigned int s0 = 0; unsigned int wait; + /* Disable PLL re-initialization during FW command processing */ + xgbe_phy_pll_ctrl(pdata, false); + /* Log if a previous command did not complete */ if (XP_IOREAD_BITS(pdata, XP_DRIVER_INT_RO, STATUS)) { netif_dbg(pdata, link, pdata->netdev, @@ -2003,7 +2017,7 @@ static void xgbe_phy_perform_ratechange(struct xgbe_prv_data *pdata, wait = XGBE_RATECHANGE_COUNT; while (wait--) { if (!XP_IOREAD_BITS(pdata, XP_DRIVER_INT_RO, STATUS)) - return; + goto reenable_pll; usleep_range(1000, 2000); } @@ -2013,6 +2027,10 @@ static void xgbe_phy_perform_ratechange(struct xgbe_prv_data *pdata, /* Reset on error */ xgbe_phy_rx_reset(pdata); + +reenable_pll: + /* Enable PLL re-initialization */ + xgbe_phy_pll_ctrl(pdata, true); } static void xgbe_phy_rrc(struct xgbe_prv_data *pdata) From e300a85db1f16a5805033feb56ec1861f02c7b1c Mon Sep 17 00:00:00 2001 From: Shuah Khan Date: Wed, 27 Oct 2021 14:28:46 -0600 Subject: [PATCH 06/17] selftests/net: update .gitignore with newly added tests Update .gitignore with newly added tests: tools/testing/selftests/net/af_unix/test_unix_oob tools/testing/selftests/net/gro tools/testing/selftests/net/ioam6_parser tools/testing/selftests/net/toeplitz Signed-off-by: Shuah Khan Signed-off-by: David S. Miller --- tools/testing/selftests/net/.gitignore | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tools/testing/selftests/net/.gitignore b/tools/testing/selftests/net/.gitignore index 19deb9cdf72f..501550501216 100644 --- a/tools/testing/selftests/net/.gitignore +++ b/tools/testing/selftests/net/.gitignore @@ -31,3 +31,7 @@ rxtimestamp timestamping txtimestamp so_netns_cookie +test_unix_oob +gro +ioam6_parser +toeplitz From f8d384a640dd32aaf0a05fec137ccbf0e986b09f Mon Sep 17 00:00:00 2001 From: Yinjun Zhang Date: Fri, 29 Oct 2021 13:29:02 +0200 Subject: [PATCH 07/17] nfp: fix NULL pointer access when scheduling dim work Each rx/tx ring has a related dim work, when rx/tx ring number is decreased by `ethtool -L`, the corresponding rx_ring or tx_ring is assigned NULL, while its related work is not destroyed. When scheduled, the work will access NULL pointer. Fixes: 9d32e4e7e9e1 ("nfp: add support for coalesce adaptive feature") Signed-off-by: Yinjun Zhang Signed-off-by: Louis Peens Signed-off-by: Simon Horman Signed-off-by: David S. Miller --- drivers/net/ethernet/netronome/nfp/nfp_net_common.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c index 5bfa22accf2c..f8b880c8e514 100644 --- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c +++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c @@ -2067,7 +2067,7 @@ static int nfp_net_poll(struct napi_struct *napi, int budget) if (napi_complete_done(napi, pkts_polled)) nfp_net_irq_unmask(r_vec->nfp_net, r_vec->irq_entry); - if (r_vec->nfp_net->rx_coalesce_adapt_on) { + if (r_vec->nfp_net->rx_coalesce_adapt_on && r_vec->rx_ring) { struct dim_sample dim_sample = {}; unsigned int start; u64 pkts, bytes; @@ -2082,7 +2082,7 @@ static int nfp_net_poll(struct napi_struct *napi, int budget) net_dim(&r_vec->rx_dim, dim_sample); } - if (r_vec->nfp_net->tx_coalesce_adapt_on) { + if (r_vec->nfp_net->tx_coalesce_adapt_on && r_vec->tx_ring) { struct dim_sample dim_sample = {}; unsigned int start; u64 pkts, bytes; From 17e712c6a1bade9dac02a7bf2b464746faa7e9a0 Mon Sep 17 00:00:00 2001 From: Yinjun Zhang Date: Fri, 29 Oct 2021 13:29:03 +0200 Subject: [PATCH 08/17] nfp: fix potential deadlock when canceling dim work When port is linked down, the process which has acquired rtnl_lock will wait for the in-progress dim work to finish, and the work also acquires rtnl_lock, which may cause deadlock. Currently IRQ_MOD registers can be configured by `ethtool -C` and dim work, and which will take effect depends on the execution order, rtnl_lock is useless here, so remove them. Fixes: 9d32e4e7e9e1 ("nfp: add support for coalesce adaptive feature") Signed-off-by: Yinjun Zhang Signed-off-by: Louis Peens Signed-off-by: Simon Horman Signed-off-by: David S. Miller --- drivers/net/ethernet/netronome/nfp/nfp_net_common.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c index f8b880c8e514..850bfdf83d0a 100644 --- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c +++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c @@ -3016,10 +3016,8 @@ static void nfp_net_rx_dim_work(struct work_struct *work) /* copy RX interrupt coalesce parameters */ value = (moder.pkts << 16) | (factor * moder.usec); - rtnl_lock(); nn_writel(nn, NFP_NET_CFG_RXR_IRQ_MOD(r_vec->rx_ring->idx), value); (void)nfp_net_reconfig(nn, NFP_NET_CFG_UPDATE_IRQMOD); - rtnl_unlock(); dim->state = DIM_START_MEASURE; } @@ -3047,10 +3045,8 @@ static void nfp_net_tx_dim_work(struct work_struct *work) /* copy TX interrupt coalesce parameters */ value = (moder.pkts << 16) | (factor * moder.usec); - rtnl_lock(); nn_writel(nn, NFP_NET_CFG_TXR_IRQ_MOD(r_vec->tx_ring->idx), value); (void)nfp_net_reconfig(nn, NFP_NET_CFG_UPDATE_IRQMOD); - rtnl_unlock(); dim->state = DIM_START_MEASURE; } From fd8d9731bcdfb22d28e45bce789bcb211c868c78 Mon Sep 17 00:00:00 2001 From: "Russell King (Oracle)" Date: Thu, 28 Oct 2021 15:55:34 +0100 Subject: [PATCH 09/17] net: phylink: avoid mvneta warning when setting pause parameters mvneta does not support asymetric pause modes, and it flags this by the lack of AsymPause in the supported field. When setting pause modes, we check that pause->rx_pause == pause->tx_pause, but only when pause autoneg is enabled. When pause autoneg is disabled, we still allow pause->rx_pause != pause->tx_pause, which is incorrect when the MAC does not support asymetric pause, and causes mvneta to issue a warning. Fix this by removing the test for pause->autoneg, so we always check that pause->rx_pause == pause->tx_pause for network devices that do not support AsymPause. Fixes: 9525ae83959b ("phylink: add phylink infrastructure") Signed-off-by: Russell King (Oracle) Signed-off-by: David S. Miller --- drivers/net/phy/phylink.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c index 0a0abe8e4be0..5defc721dd05 100644 --- a/drivers/net/phy/phylink.c +++ b/drivers/net/phy/phylink.c @@ -1724,7 +1724,7 @@ int phylink_ethtool_set_pauseparam(struct phylink *pl, return -EOPNOTSUPP; if (!phylink_test(pl->supported, Asym_Pause) && - !pause->autoneg && pause->rx_pause != pause->tx_pause) + pause->rx_pause != pause->tx_pause) return -EINVAL; pause_state = 0; From 829e050eea69c7442441b714b6f5b339b5b8c367 Mon Sep 17 00:00:00 2001 From: Ivan Vecera Date: Thu, 28 Oct 2021 17:58:35 +0200 Subject: [PATCH 10/17] net: bridge: fix uninitialized variables when BRIDGE_CFM is disabled Function br_get_link_af_size_filtered() calls br_cfm_{,peer}_mep_count() that return a count. When BRIDGE_CFM is not enabled these functions simply return -EOPNOTSUPP but do not modify count parameter and calling function then works with uninitialized variables. Modify these inline functions to return zero in count parameter. Fixes: b6d0425b816e ("bridge: cfm: Netlink Notifications.") Cc: Henrik Bjoernlund Signed-off-by: Ivan Vecera Acked-by: Nikolay Aleksandrov Signed-off-by: David S. Miller --- net/bridge/br_private.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h index 37ca76406f1e..fd5e7e74573c 100644 --- a/net/bridge/br_private.h +++ b/net/bridge/br_private.h @@ -1911,11 +1911,13 @@ static inline int br_cfm_status_fill_info(struct sk_buff *skb, static inline int br_cfm_mep_count(struct net_bridge *br, u32 *count) { + *count = 0; return -EOPNOTSUPP; } static inline int br_cfm_peer_mep_count(struct net_bridge *br, u32 *count) { + *count = 0; return -EOPNOTSUPP; } #endif From 34d7ecb3d4f772eb00ce1f7195ae30886ddf4d2e Mon Sep 17 00:00:00 2001 From: Nikolay Aleksandrov Date: Fri, 29 Oct 2021 15:05:27 +0300 Subject: [PATCH 11/17] selftests: net: bridge: update IGMP/MLD membership interval value When I fixed IGMPv3/MLDv2 to use the bridge's multicast_membership_interval value which is chosen by user-space instead of calculating it based on multicast_query_interval and multicast_query_response_interval I forgot to update the selftests relying on that behaviour. Now we have to manually set the expected GMI value to perform the tests correctly and get proper results (similar to IGMPv2 behaviour). Fixes: fac3cb82a54a ("net: bridge: mcast: use multicast_membership_interval for IGMPv3") Signed-off-by: Nikolay Aleksandrov Signed-off-by: David S. Miller --- .../testing/selftests/net/forwarding/bridge_igmp.sh | 12 +++++++++--- tools/testing/selftests/net/forwarding/bridge_mld.sh | 12 +++++++++--- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/tools/testing/selftests/net/forwarding/bridge_igmp.sh b/tools/testing/selftests/net/forwarding/bridge_igmp.sh index 675eff45b037..1162836f8f32 100755 --- a/tools/testing/selftests/net/forwarding/bridge_igmp.sh +++ b/tools/testing/selftests/net/forwarding/bridge_igmp.sh @@ -482,10 +482,15 @@ v3exc_timeout_test() local X=("192.0.2.20" "192.0.2.30") # GMI should be 3 seconds - ip link set dev br0 type bridge mcast_query_interval 100 mcast_query_response_interval 100 + ip link set dev br0 type bridge mcast_query_interval 100 \ + mcast_query_response_interval 100 \ + mcast_membership_interval 300 v3exclude_prepare $h1 $ALL_MAC $ALL_GROUP - ip link set dev br0 type bridge mcast_query_interval 500 mcast_query_response_interval 500 + ip link set dev br0 type bridge mcast_query_interval 500 \ + mcast_query_response_interval 500 \ + mcast_membership_interval 1500 + $MZ $h1 -c 1 -b $ALL_MAC -B $ALL_GROUP -t ip "proto=2,p=$MZPKT_ALLOW2" -q sleep 3 bridge -j -d -s mdb show dev br0 \ @@ -517,7 +522,8 @@ v3exc_timeout_test() log_test "IGMPv3 group $TEST_GROUP exclude timeout" ip link set dev br0 type bridge mcast_query_interval 12500 \ - mcast_query_response_interval 1000 + mcast_query_response_interval 1000 \ + mcast_membership_interval 26000 v3cleanup $swp1 $TEST_GROUP } diff --git a/tools/testing/selftests/net/forwarding/bridge_mld.sh b/tools/testing/selftests/net/forwarding/bridge_mld.sh index ffdcfa87ca2b..e2b9ff773c6b 100755 --- a/tools/testing/selftests/net/forwarding/bridge_mld.sh +++ b/tools/testing/selftests/net/forwarding/bridge_mld.sh @@ -479,10 +479,15 @@ mldv2exc_timeout_test() local X=("2001:db8:1::20" "2001:db8:1::30") # GMI should be 3 seconds - ip link set dev br0 type bridge mcast_query_interval 100 mcast_query_response_interval 100 + ip link set dev br0 type bridge mcast_query_interval 100 \ + mcast_query_response_interval 100 \ + mcast_membership_interval 300 mldv2exclude_prepare $h1 - ip link set dev br0 type bridge mcast_query_interval 500 mcast_query_response_interval 500 + ip link set dev br0 type bridge mcast_query_interval 500 \ + mcast_query_response_interval 500 \ + mcast_membership_interval 1500 + $MZ $h1 -c 1 $MZPKT_ALLOW2 -q sleep 3 bridge -j -d -s mdb show dev br0 \ @@ -514,7 +519,8 @@ mldv2exc_timeout_test() log_test "MLDv2 group $TEST_GROUP exclude timeout" ip link set dev br0 type bridge mcast_query_interval 12500 \ - mcast_query_response_interval 1000 + mcast_query_response_interval 1000 \ + mcast_membership_interval 26000 mldv2cleanup $swp1 } From 6de6e46d27ef386feecdbea56b3bfd6c3b3bc1f9 Mon Sep 17 00:00:00 2001 From: Yoshiki Komachi Date: Fri, 29 Oct 2021 09:21:41 +0000 Subject: [PATCH 12/17] cls_flower: Fix inability to match GRE/IPIP packets When a packet of a new flow arrives in openvswitch kernel module, it dissects the packet and passes the extracted flow key to ovs-vswtichd daemon. If hw- offload configuration is enabled, the daemon creates a new TC flower entry to bypass openvswitch kernel module for the flow (TC flower can also offload flows to NICs but this time that does not matter). In this processing flow, I found the following issue in cases of GRE/IPIP packets. When ovs_flow_key_extract() in openvswitch module parses a packet of a new GRE (or IPIP) flow received on non-tunneling vports, it extracts information of the outer IP header for ip_proto/src_ip/dst_ip match keys. This means ovs-vswitchd creates a TC flower entry with IP protocol/addresses match keys whose values are those of the outer IP header. OTOH, TC flower, which uses flow_dissector (different parser from openvswitch module), extracts information of the inner IP header. The following flow is an example to describe the issue in more detail. <----------- Outer IP -----------------> <---------- Inner IP ----------> +----------+--------------+--------------+----------+----------+----------+ | ip_proto | src_ip | dst_ip | ip_proto | src_ip | dst_ip | | 47 (GRE) | 192.168.10.1 | 192.168.10.2 | 6 (TCP) | 10.0.0.1 | 10.0.0.2 | +----------+--------------+--------------+----------+----------+----------+ In this case, TC flower entry and extracted information are shown as below: - ovs-vswitchd creates TC flower entry with: - ip_proto: 47 - src_ip: 192.168.10.1 - dst_ip: 192.168.10.2 - TC flower extracts below for IP header matches: - ip_proto: 6 - src_ip: 10.0.0.1 - dst_ip: 10.0.0.2 Thus, GRE or IPIP packets never match the TC flower entry, as each dissector behaves differently. IMHO, the behavior of TC flower (flow dissector) does not look correct, as ip_proto/src_ip/dst_ip in TC flower match means the outermost IP header information except for GRE/IPIP cases. This patch adds a new flow_dissector flag FLOW_DISSECTOR_F_STOP_BEFORE_ENCAP which skips dissection of the encapsulated inner GRE/IPIP header in TC flower classifier. Signed-off-by: Yoshiki Komachi Signed-off-by: David S. Miller --- include/net/flow_dissector.h | 1 + net/core/flow_dissector.c | 15 +++++++++++++++ net/sched/cls_flower.c | 3 ++- 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h index ffd386ea0dbb..aa33e1092e2c 100644 --- a/include/net/flow_dissector.h +++ b/include/net/flow_dissector.h @@ -287,6 +287,7 @@ enum flow_dissector_key_id { #define FLOW_DISSECTOR_F_PARSE_1ST_FRAG BIT(0) #define FLOW_DISSECTOR_F_STOP_AT_FLOW_LABEL BIT(1) #define FLOW_DISSECTOR_F_STOP_AT_ENCAP BIT(2) +#define FLOW_DISSECTOR_F_STOP_BEFORE_ENCAP BIT(3) struct flow_dissector_key { enum flow_dissector_key_id key_id; diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c index bac0184cf3de..0d4bbf534c7d 100644 --- a/net/core/flow_dissector.c +++ b/net/core/flow_dissector.c @@ -1307,6 +1307,11 @@ ip_proto_again: switch (ip_proto) { case IPPROTO_GRE: + if (flags & FLOW_DISSECTOR_F_STOP_BEFORE_ENCAP) { + fdret = FLOW_DISSECT_RET_OUT_GOOD; + break; + } + fdret = __skb_flow_dissect_gre(skb, key_control, flow_dissector, target_container, data, &proto, &nhoff, &hlen, flags); @@ -1364,6 +1369,11 @@ ip_proto_again: break; } case IPPROTO_IPIP: + if (flags & FLOW_DISSECTOR_F_STOP_BEFORE_ENCAP) { + fdret = FLOW_DISSECT_RET_OUT_GOOD; + break; + } + proto = htons(ETH_P_IP); key_control->flags |= FLOW_DIS_ENCAPSULATION; @@ -1376,6 +1386,11 @@ ip_proto_again: break; case IPPROTO_IPV6: + if (flags & FLOW_DISSECTOR_F_STOP_BEFORE_ENCAP) { + fdret = FLOW_DISSECT_RET_OUT_GOOD; + break; + } + proto = htons(ETH_P_IPV6); key_control->flags |= FLOW_DIS_ENCAPSULATION; diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c index eb6345a027e1..aab13ba11767 100644 --- a/net/sched/cls_flower.c +++ b/net/sched/cls_flower.c @@ -329,7 +329,8 @@ static int fl_classify(struct sk_buff *skb, const struct tcf_proto *tp, ARRAY_SIZE(fl_ct_info_to_flower_map), post_ct); skb_flow_dissect_hash(skb, &mask->dissector, &skb_key); - skb_flow_dissect(skb, &mask->dissector, &skb_key, 0); + skb_flow_dissect(skb, &mask->dissector, &skb_key, + FLOW_DISSECTOR_F_STOP_BEFORE_ENCAP); f = fl_mask_lookup(mask, &skb_key); if (f && !tc_skip_sw(f->flags)) { From 42dcfd850e514b229d616a53dec06d0f2533217c Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Fri, 29 Oct 2021 08:51:34 -0700 Subject: [PATCH 13/17] udp6: allow SO_MARK ctrl msg to affect routing Commit c6af0c227a22 ("ip: support SO_MARK cmsg") added propagation of SO_MARK from cmsg to skb->mark. For IPv4 and raw sockets the mark also affects route lookup, but in case of IPv6 the flow info is initialized before cmsg is parsed. Fixes: c6af0c227a22 ("ip: support SO_MARK cmsg") Reported-and-tested-by: Xintong Hu Signed-off-by: Jakub Kicinski Reviewed-by: David Ahern Reviewed-by: Willem de Bruijn Signed-off-by: David S. Miller --- net/ipv6/udp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c index 8d785232b479..be6dc64ece29 100644 --- a/net/ipv6/udp.c +++ b/net/ipv6/udp.c @@ -1435,7 +1435,6 @@ do_udp_sendmsg: if (!fl6.flowi6_oif) fl6.flowi6_oif = np->sticky_pktinfo.ipi6_ifindex; - fl6.flowi6_mark = ipc6.sockc.mark; fl6.flowi6_uid = sk->sk_uid; if (msg->msg_controllen) { @@ -1471,6 +1470,7 @@ do_udp_sendmsg: ipc6.opt = opt; fl6.flowi6_proto = sk->sk_protocol; + fl6.flowi6_mark = ipc6.sockc.mark; fl6.daddr = *daddr; if (ipv6_addr_any(&fl6.saddr) && !ipv6_addr_any(&np->saddr)) fl6.saddr = np->saddr; From b0ced8f290fb7bb03d23c4c3c3355e92a4be6e95 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Fri, 29 Oct 2021 08:51:35 -0700 Subject: [PATCH 14/17] selftests: udp: test for passing SO_MARK as cmsg Before fix: | Case IPv6 rejection returned 0, expected 1 |FAIL - 1/4 cases failed With the fix: | OK Signed-off-by: Jakub Kicinski Reviewed-by: David Ahern Reviewed-by: Willem de Bruijn Signed-off-by: David S. Miller --- tools/testing/selftests/net/.gitignore | 1 + tools/testing/selftests/net/Makefile | 2 + tools/testing/selftests/net/cmsg_so_mark.c | 67 +++++++++++++++++++++ tools/testing/selftests/net/cmsg_so_mark.sh | 61 +++++++++++++++++++ 4 files changed, 131 insertions(+) create mode 100644 tools/testing/selftests/net/cmsg_so_mark.c create mode 100755 tools/testing/selftests/net/cmsg_so_mark.sh diff --git a/tools/testing/selftests/net/.gitignore b/tools/testing/selftests/net/.gitignore index 501550501216..7581a7348e1b 100644 --- a/tools/testing/selftests/net/.gitignore +++ b/tools/testing/selftests/net/.gitignore @@ -35,3 +35,4 @@ test_unix_oob gro ioam6_parser toeplitz +cmsg_so_mark diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile index 492b273743b4..f56b652d5cc6 100644 --- a/tools/testing/selftests/net/Makefile +++ b/tools/testing/selftests/net/Makefile @@ -28,6 +28,7 @@ TEST_PROGS += veth.sh TEST_PROGS += ioam6.sh TEST_PROGS += gro.sh TEST_PROGS += gre_gso.sh +TEST_PROGS += cmsg_so_mark.sh TEST_PROGS_EXTENDED := in_netns.sh TEST_GEN_FILES = socket nettest TEST_GEN_FILES += psock_fanout psock_tpacket msg_zerocopy reuseport_addr_any @@ -44,6 +45,7 @@ TEST_GEN_FILES += gro TEST_GEN_PROGS = reuseport_bpf reuseport_bpf_cpu reuseport_bpf_numa TEST_GEN_PROGS += reuseport_dualstack reuseaddr_conflict tls TEST_GEN_FILES += toeplitz +TEST_GEN_FILES += cmsg_so_mark TEST_FILES := settings diff --git a/tools/testing/selftests/net/cmsg_so_mark.c b/tools/testing/selftests/net/cmsg_so_mark.c new file mode 100644 index 000000000000..27f2804892a7 --- /dev/null +++ b/tools/testing/selftests/net/cmsg_so_mark.c @@ -0,0 +1,67 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +#include +#include +#include +#include +#include +#include +#include +#include + +int main(int argc, const char **argv) +{ + char cbuf[CMSG_SPACE(sizeof(__u32))]; + struct addrinfo hints, *ai; + struct cmsghdr *cmsg; + struct iovec iov[1]; + struct msghdr msg; + int mark; + int err; + int fd; + + if (argc != 4) { + fprintf(stderr, "Usage: %s \n", argv[0]); + return 1; + } + mark = atoi(argv[3]); + + memset(&hints, 0, sizeof(hints)); + hints.ai_family = AF_UNSPEC; + hints.ai_socktype = SOCK_DGRAM; + + ai = NULL; + err = getaddrinfo(argv[1], argv[2], &hints, &ai); + if (err) { + fprintf(stderr, "Can't resolve address: %s\n", strerror(errno)); + return 1; + } + + fd = socket(ai->ai_family, SOCK_DGRAM, IPPROTO_UDP); + if (fd < 0) { + fprintf(stderr, "Can't open socket: %s\n", strerror(errno)); + freeaddrinfo(ai); + return 1; + } + + iov[0].iov_base = "bla"; + iov[0].iov_len = 4; + + msg.msg_name = ai->ai_addr; + msg.msg_namelen = ai->ai_addrlen; + msg.msg_iov = iov; + msg.msg_iovlen = 1; + msg.msg_control = cbuf; + msg.msg_controllen = sizeof(cbuf); + + cmsg = CMSG_FIRSTHDR(&msg); + cmsg->cmsg_level = SOL_SOCKET; + cmsg->cmsg_type = SO_MARK; + cmsg->cmsg_len = CMSG_LEN(sizeof(__u32)); + *(__u32 *)CMSG_DATA(cmsg) = mark; + + err = sendmsg(fd, &msg, 0); + + close(fd); + freeaddrinfo(ai); + return err != 4; +} diff --git a/tools/testing/selftests/net/cmsg_so_mark.sh b/tools/testing/selftests/net/cmsg_so_mark.sh new file mode 100755 index 000000000000..19c6aab8d0e9 --- /dev/null +++ b/tools/testing/selftests/net/cmsg_so_mark.sh @@ -0,0 +1,61 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0 + +NS=ns +IP4=172.16.0.1/24 +TGT4=172.16.0.2 +IP6=2001:db8:1::1/64 +TGT6=2001:db8:1::2 +MARK=1000 + +cleanup() +{ + ip netns del $NS +} + +trap cleanup EXIT + +# Namespaces +ip netns add $NS + +# Connectivity +ip -netns $NS link add type dummy +ip -netns $NS link set dev dummy0 up +ip -netns $NS addr add $IP4 dev dummy0 +ip -netns $NS addr add $IP6 dev dummy0 + +ip -netns $NS rule add fwmark $MARK lookup 300 +ip -6 -netns $NS rule add fwmark $MARK lookup 300 +ip -netns $NS route add prohibit any table 300 +ip -6 -netns $NS route add prohibit any table 300 + +# Test +BAD=0 +TOTAL=0 + +check_result() { + ((TOTAL++)) + if [ $1 -ne $2 ]; then + echo " Case $3 returned $1, expected $2" + ((BAD++)) + fi +} + +ip netns exec $NS ./cmsg_so_mark $TGT4 1234 $((MARK + 1)) +check_result $? 0 "IPv4 pass" +ip netns exec $NS ./cmsg_so_mark $TGT6 1234 $((MARK + 1)) +check_result $? 0 "IPv6 pass" + +ip netns exec $NS ./cmsg_so_mark $TGT4 1234 $MARK +check_result $? 1 "IPv4 rejection" +ip netns exec $NS ./cmsg_so_mark $TGT6 1234 $MARK +check_result $? 1 "IPv6 rejection" + +# Summary +if [ $BAD -ne 0 ]; then + echo "FAIL - $BAD/$TOTAL cases failed" + exit 1 +else + echo "OK" + exit 0 +fi From 8878e46fcfd46b19964bd90e13b25dd94cbfc9be Mon Sep 17 00:00:00 2001 From: Sukadev Bhattiprolu Date: Fri, 29 Oct 2021 15:03:14 -0700 Subject: [PATCH 15/17] ibmvnic: don't stop queue in xmit If adapter's resetting bit is on, discard the packet but don't stop the transmit queue - instead leave that to the reset code. With this change, it is possible that we may get several calls to ibmvnic_xmit() that simply discard packets and return. But if we stop the queue here, we might end up doing so just after __ibmvnic_open() started the queues (during a hard/soft reset) and before the ->resetting bit was cleared. If that happens, there will be no one to restart queue and transmissions will be blocked indefinitely. This can cause a TIMEOUT reset and with auto priority failover enabled, an unnecessary FAILOVER reset to less favored backing device and then a FAILOVER back to the most favored backing device. If we hit the window repeatedly, we can get stuck in a loop of TIMEOUT, FAILOVER, FAILOVER resets leaving the adapter unusable for extended periods of time. Fixes: 7f5b030830fe ("ibmvnic: Free skb's in cases of failure in transmit") Reported-by: Abdul Haleem Reported-by: Vaishnavi Bhat Signed-off-by: Sukadev Bhattiprolu Reviewed-by: Dany Madden Signed-off-by: David S. Miller --- drivers/net/ethernet/ibm/ibmvnic.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index 6aa6ff89a765..7438138c3766 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -1724,8 +1724,6 @@ static netdev_tx_t ibmvnic_xmit(struct sk_buff *skb, struct net_device *netdev) ind_bufp = &tx_scrq->ind_buf; if (test_bit(0, &adapter->resetting)) { - if (!netif_subqueue_stopped(netdev, skb)) - netif_stop_subqueue(netdev, queue_num); dev_kfree_skb_any(skb); tx_send_failed++; From 6e20d00158f31f7631d68b86996b7e951c4451c8 Mon Sep 17 00:00:00 2001 From: Sukadev Bhattiprolu Date: Fri, 29 Oct 2021 15:03:15 -0700 Subject: [PATCH 16/17] ibmvnic: Process crqs after enabling interrupts Soon after registering a CRQ it is possible that we get a fail over or maybe a CRQ_INIT from the VIOS while interrupts were disabled. Look for any such CRQs after enabling interrupts. Otherwise we can intermittently fail to bring up ibmvnic adapters during boot, specially in kexec/kdump kernels. Fixes: 032c5e82847a ("Driver for IBM System i/p VNIC protocol") Reported-by: Vaishnavi Bhat Signed-off-by: Sukadev Bhattiprolu Reviewed-by: Dany Madden Signed-off-by: David S. Miller --- drivers/net/ethernet/ibm/ibmvnic.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index 7438138c3766..84961a83803b 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -5412,6 +5412,9 @@ static int init_crq_queue(struct ibmvnic_adapter *adapter) crq->cur = 0; spin_lock_init(&crq->lock); + /* process any CRQs that were queued before we enabled interrupts */ + tasklet_schedule(&adapter->tasklet); + return retrc; req_irq_failed: From 6b278c0cb378079f3c0c61ae4a369c09ff1a4188 Mon Sep 17 00:00:00 2001 From: Sukadev Bhattiprolu Date: Fri, 29 Oct 2021 15:03:16 -0700 Subject: [PATCH 17/17] ibmvnic: delay complete() If we get CRQ_INIT, we set errno to -EIO and first call complete() to notify the waiter. Then we try to schedule a FAILOVER reset. If this occurs while adapter is in PROBING state, ibmvnic_reset() changes the error code to EAGAIN and returns without scheduling the FAILOVER. The purpose of setting error code to EAGAIN is to ask the waiter to retry. But due to the earlier complete() call, the waiter may already have seen the -EIO response and decided not to retry. This can cause intermittent failures when bringing up ibmvnic adapters during boot, specially in in kexec/kdump kernels. Defer the complete() call until after scheduling the reset. Also streamline the error code to EAGAIN. Don't see why we need EIO sometimes. All 3 callers of ibmvnic_reset_init() can handle EAGAIN. Fixes: 17c8705838a5 ("ibmvnic: Return error code if init interrupted by transport event") Reported-by: Vaishnavi Bhat Signed-off-by: Sukadev Bhattiprolu Reviewed-by: Dany Madden Signed-off-by: David S. Miller --- drivers/net/ethernet/ibm/ibmvnic.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index 84961a83803b..352ffe982d84 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -2565,7 +2565,7 @@ static int ibmvnic_reset(struct ibmvnic_adapter *adapter, if (adapter->state == VNIC_PROBING) { netdev_warn(netdev, "Adapter reset during probe\n"); - adapter->init_done_rc = EAGAIN; + adapter->init_done_rc = -EAGAIN; ret = EAGAIN; goto err; } @@ -5067,11 +5067,6 @@ static void ibmvnic_handle_crq(union ibmvnic_crq *crq, */ adapter->login_pending = false; - if (!completion_done(&adapter->init_done)) { - complete(&adapter->init_done); - adapter->init_done_rc = -EIO; - } - if (adapter->state == VNIC_DOWN) rc = ibmvnic_reset(adapter, VNIC_RESET_PASSIVE_INIT); else @@ -5092,6 +5087,13 @@ static void ibmvnic_handle_crq(union ibmvnic_crq *crq, rc); adapter->failover_pending = false; } + + if (!completion_done(&adapter->init_done)) { + complete(&adapter->init_done); + if (!adapter->init_done_rc) + adapter->init_done_rc = -EAGAIN; + } + break; case IBMVNIC_CRQ_INIT_COMPLETE: dev_info(dev, "Partner initialization complete\n"); @@ -5559,7 +5561,7 @@ static int ibmvnic_probe(struct vio_dev *dev, const struct vio_device_id *id) } rc = ibmvnic_reset_init(adapter, false); - } while (rc == EAGAIN); + } while (rc == -EAGAIN); /* We are ignoring the error from ibmvnic_reset_init() assuming that the * partner is not ready. CRQ is not active. When the partner becomes