Date: Mon, 17 Apr 2023 16:09:30 GMT From: Gleb Smirnoff <glebius@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: a6b55ee6be15 - main - net: replace IFF_KNOWSEPOCH with IFF_NEEDSEPOCH Message-ID: <202304171609.33HG9UOp003907@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by glebius: URL: https://cgit.FreeBSD.org/src/commit/?id=a6b55ee6be15a41792839095d19b589e25d0f7f7 commit a6b55ee6be15a41792839095d19b589e25d0f7f7 Author: Gleb Smirnoff <glebius@FreeBSD.org> AuthorDate: 2023-04-17 16:08:35 +0000 Commit: Gleb Smirnoff <glebius@FreeBSD.org> CommitDate: 2023-04-17 16:08:35 +0000 net: replace IFF_KNOWSEPOCH with IFF_NEEDSEPOCH Expect that drivers call into the network stack with the net epoch entered. This has already been the fact since early 2020. The net interrupts, that are marked with INTR_TYPE_NET, were entering epoch since 511d1afb6bf. For the taskqueues there is NET_TASK_INIT() and all drivers that were known back in 2020 we marked with it in 6c3e93cb5a4. However in e87c4940156 we took conservative approach and preferred to opt-in rather than opt-out for the epoch. This change not only reverts e87c4940156 but adds a safety belt to avoid panicing with INVARIANTS if there is a missed driver. With INVARIANTS we will run in_epoch() check, print a warning and enter the net epoch. A driver that prints can be quickly fixed with the IFF_NEEDSEPOCH flag, but better be augmented to properly enter the epoch itself. Note on TCP LRO: it is a backdoor to enter the TCP stack bypassing some layers of net stack, ignoring either old IFF_KNOWSEPOCH or the new IFF_NEEDSEPOCH. But the tcp_lro_flush_all() asserts the presence of network epoch. Indeed, all NIC drivers that support LRO already provide the epoch, either with help of INTR_TYPE_NET or just running NET_EPOCH_ENTER() in their code. Reviewed by: zlei, gallatin, erj Differential Revision: https://reviews.freebsd.org/D39510 --- sys/compat/linux/linux_netlink.c | 2 +- sys/dev/ena/ena.c | 3 +-- sys/dev/mlx5/mlx5_en/mlx5_en_main.c | 3 +-- sys/dev/oce/oce_if.c | 2 +- sys/dev/virtio/network/if_vtnet.c | 3 +-- sys/net/if.h | 4 ++-- sys/net/if_epair.c | 1 - sys/net/if_ethersubr.c | 25 ++++++++++++++++++++- sys/net/if_infiniband.c | 26 +++++++++++++++++++++- sys/net/iflib.c | 2 +- sys/ofed/drivers/infiniband/ulp/ipoib/ipoib_main.c | 4 ++-- 11 files changed, 59 insertions(+), 16 deletions(-) diff --git a/sys/compat/linux/linux_netlink.c b/sys/compat/linux/linux_netlink.c index 0e8188d4cdf6..775a36994d2d 100644 --- a/sys/compat/linux/linux_netlink.c +++ b/sys/compat/linux/linux_netlink.c @@ -312,7 +312,7 @@ rtnl_if_flags_to_linux(unsigned int if_flags) case IFF_ALLMULTI: result |= flag; break; - case IFF_KNOWSEPOCH: + case IFF_NEEDSEPOCH: case IFF_DRV_OACTIVE: case IFF_SIMPLEX: case IFF_LINK0: diff --git a/sys/dev/ena/ena.c b/sys/dev/ena/ena.c index 72846a8bed51..a4762ce9ebb1 100644 --- a/sys/dev/ena/ena.c +++ b/sys/dev/ena/ena.c @@ -2393,8 +2393,7 @@ ena_setup_ifnet(device_t pdev, struct ena_adapter *adapter, if_setdev(ifp, pdev); if_setsoftc(ifp, adapter); - if_setflags(ifp, - IFF_BROADCAST | IFF_SIMPLEX | IFF_MULTICAST | IFF_KNOWSEPOCH); + if_setflags(ifp, IFF_BROADCAST | IFF_SIMPLEX | IFF_MULTICAST); if_setinitfn(ifp, ena_init); if_settransmitfn(ifp, ena_mq_start); if_setqflushfn(ifp, ena_qflush); diff --git a/sys/dev/mlx5/mlx5_en/mlx5_en_main.c b/sys/dev/mlx5/mlx5_en/mlx5_en_main.c index 84adef8398bb..ab0cf49c2e8a 100644 --- a/sys/dev/mlx5/mlx5_en/mlx5_en_main.c +++ b/sys/dev/mlx5/mlx5_en/mlx5_en_main.c @@ -4526,8 +4526,7 @@ mlx5e_create_ifp(struct mlx5_core_dev *mdev) if_initname(ifp, "mce", device_get_unit(mdev->pdev->dev.bsddev)); if_setmtu(ifp, ETHERMTU); if_setinitfn(ifp, mlx5e_open); - if_setflags(ifp, IFF_BROADCAST | IFF_SIMPLEX | IFF_MULTICAST | - IFF_KNOWSEPOCH); + if_setflags(ifp, IFF_BROADCAST | IFF_SIMPLEX | IFF_MULTICAST); if_setioctlfn(ifp, mlx5e_ioctl); if_settransmitfn(ifp, mlx5e_xmit); if_setqflushfn(ifp, if_qflush); diff --git a/sys/dev/oce/oce_if.c b/sys/dev/oce/oce_if.c index cc8cfc3eaa8c..5d250fcac0bd 100644 --- a/sys/dev/oce/oce_if.c +++ b/sys/dev/oce/oce_if.c @@ -2111,7 +2111,7 @@ oce_attach_ifp(POCE_SOFTC sc) ifmedia_add(&sc->media, IFM_ETHER | IFM_AUTO, 0, NULL); ifmedia_set(&sc->media, IFM_ETHER | IFM_AUTO); - if_setflags(sc->ifp, IFF_BROADCAST | IFF_MULTICAST | IFF_KNOWSEPOCH); + if_setflags(sc->ifp, IFF_BROADCAST | IFF_MULTICAST); if_setioctlfn(sc->ifp, oce_ioctl); if_setstartfn(sc->ifp, oce_start); if_setinitfn(sc->ifp, oce_init); diff --git a/sys/dev/virtio/network/if_vtnet.c b/sys/dev/virtio/network/if_vtnet.c index 41eaa6a56086..9ef667e97a54 100644 --- a/sys/dev/virtio/network/if_vtnet.c +++ b/sys/dev/virtio/network/if_vtnet.c @@ -1103,8 +1103,7 @@ vtnet_setup_interface(struct vtnet_softc *sc) dev = sc->vtnet_dev; ifp = sc->vtnet_ifp; - if_setflags(ifp, IFF_BROADCAST | IFF_SIMPLEX | IFF_MULTICAST | - IFF_KNOWSEPOCH); + if_setflags(ifp, IFF_BROADCAST | IFF_SIMPLEX | IFF_MULTICAST); if_setbaudrate(ifp, IF_Gbps(10)); if_setinitfn(ifp, vtnet_init); if_setioctlfn(ifp, vtnet_ioctl); diff --git a/sys/net/if.h b/sys/net/if.h index 888e7d5d7320..da3d25f2b226 100644 --- a/sys/net/if.h +++ b/sys/net/if.h @@ -144,7 +144,7 @@ struct if_data { #define IFF_DEBUG 0x4 /* (n) turn on debugging */ #define IFF_LOOPBACK 0x8 /* (i) is a loopback net */ #define IFF_POINTOPOINT 0x10 /* (i) is a point-to-point link */ -#define IFF_KNOWSEPOCH 0x20 /* (i) calls if_input in net epoch */ +#define IFF_NEEDSEPOCH 0x20 /* (i) calls if_input w/o net epoch */ #define IFF_DRV_RUNNING 0x40 /* (d) resources allocated */ #define IFF_NOARP 0x80 /* (n) no address resolution protocol */ #define IFF_PROMISC 0x100 /* (n) receive all packets */ @@ -179,7 +179,7 @@ struct if_data { #define IFF_CANTCHANGE \ (IFF_BROADCAST|IFF_POINTOPOINT|IFF_DRV_RUNNING|IFF_DRV_OACTIVE|\ IFF_SIMPLEX|IFF_MULTICAST|IFF_ALLMULTI|IFF_PROMISC|\ - IFF_DYING|IFF_CANTCONFIG|IFF_KNOWSEPOCH) + IFF_DYING|IFF_CANTCONFIG|IFF_NEEDSEPOCH) /* * Values for if_link_state. diff --git a/sys/net/if_epair.c b/sys/net/if_epair.c index e9e1a48b3d58..2afbf786c9c8 100644 --- a/sys/net/if_epair.c +++ b/sys/net/if_epair.c @@ -543,7 +543,6 @@ epair_setup_ifp(struct epair_softc *sc, char *name, int unit) ifp->if_dname = epairname; ifp->if_dunit = unit; ifp->if_flags = IFF_BROADCAST | IFF_SIMPLEX | IFF_MULTICAST; - ifp->if_flags |= IFF_KNOWSEPOCH; ifp->if_capabilities = IFCAP_VLAN_MTU; ifp->if_capenable = IFCAP_VLAN_MTU; ifp->if_transmit = epair_transmit; diff --git a/sys/net/if_ethersubr.c b/sys/net/if_ethersubr.c index 839bae8e9d43..dd5c07acf634 100644 --- a/sys/net/if_ethersubr.c +++ b/sys/net/if_ethersubr.c @@ -56,6 +56,9 @@ #include <sys/sockio.h> #include <sys/sysctl.h> #include <sys/uuid.h> +#ifdef KDB +#include <sys/kdb.h> +#endif #include <net/ieee_oui.h> #include <net/if.h> @@ -813,7 +816,27 @@ ether_input(struct ifnet *ifp, struct mbuf *m) struct mbuf *mn; bool needs_epoch; - needs_epoch = !(ifp->if_flags & IFF_KNOWSEPOCH); + needs_epoch = (ifp->if_flags & IFF_NEEDSEPOCH); +#ifdef INVARIANTS + /* + * This temporary code is here to prevent epoch unaware and unmarked + * drivers to panic the system. Once all drivers are taken care of, + * the whole INVARIANTS block should go away. + */ + if (!needs_epoch && !in_epoch(net_epoch_preempt)) { + static bool printedonce; + + needs_epoch = true; + if (!printedonce) { + printedonce = true; + if_printf(ifp, "called %s w/o net epoch! " + "PLEASE file a bug report.", __func__); +#ifdef KDB + kdb_backtrace(); +#endif + } + } +#endif /* * The drivers are allowed to pass in a chain of packets linked with diff --git a/sys/net/if_infiniband.c b/sys/net/if_infiniband.c index 30f014ee669d..a11b8a8f5c56 100644 --- a/sys/net/if_infiniband.c +++ b/sys/net/if_infiniband.c @@ -25,6 +25,7 @@ #include "opt_inet.h" #include "opt_inet6.h" +#include "opt_kbd.h" #include <sys/cdefs.h> __FBSDID("$FreeBSD$"); @@ -38,6 +39,9 @@ __FBSDID("$FreeBSD$"); #include <sys/module.h> #include <sys/socket.h> #include <sys/sysctl.h> +#ifdef KDB +#include <sys/kdb.h> +#endif #include <net/bpf.h> #include <net/ethernet.h> @@ -417,7 +421,27 @@ infiniband_input(struct ifnet *ifp, struct mbuf *m) int isr; bool needs_epoch; - needs_epoch = (ifp->if_flags & IFF_KNOWSEPOCH) == 0; + needs_epoch = (ifp->if_flags & IFF_NEEDSEPOCH); +#ifdef INVARIANTS + /* + * This temporary code is here to prevent epoch unaware and unmarked + * drivers to panic the system. Once all drivers are taken care of, + * the whole INVARIANTS block should go away. + */ + if (!needs_epoch && !in_epoch(net_epoch_preempt)) { + static bool printedonce; + + needs_epoch = true; + if (!printedonce) { + printedonce = true; + if_printf(ifp, "called %s w/o net epoch! " + "PLEASE file a bug report.", __func__); +#ifdef KDB + kdb_backtrace(); +#endif + } + } +#endif CURVNET_SET_QUIET(ifp->if_vnet); if (__predict_false(needs_epoch)) diff --git a/sys/net/iflib.c b/sys/net/iflib.c index aa16e5d5492b..d056570d9a99 100644 --- a/sys/net/iflib.c +++ b/sys/net/iflib.c @@ -6010,7 +6010,7 @@ iflib_register(if_ctx_t ctx) if_settransmitfn(ifp, iflib_if_transmit); #endif if_setqflushfn(ifp, iflib_if_qflush); - iflags = IFF_MULTICAST | IFF_KNOWSEPOCH; + iflags = IFF_MULTICAST; if ((sctx->isc_flags & IFLIB_PSEUDO) && (sctx->isc_flags & IFLIB_PSEUDO_ETHER) == 0) diff --git a/sys/ofed/drivers/infiniband/ulp/ipoib/ipoib_main.c b/sys/ofed/drivers/infiniband/ulp/ipoib/ipoib_main.c index 8b2f4724d2fd..fe4581e456d3 100644 --- a/sys/ofed/drivers/infiniband/ulp/ipoib/ipoib_main.c +++ b/sys/ofed/drivers/infiniband/ulp/ipoib/ipoib_main.c @@ -922,8 +922,8 @@ ipoib_intf_alloc(const char *name, struct ib_device *hca) } if_initname(dev, name, priv->unit); if_setflags(dev, IFF_BROADCAST | IFF_MULTICAST); - if (hca->attrs.device_cap_flags & IB_DEVICE_KNOWSEPOCH) - if_setflagbits(dev, IFF_KNOWSEPOCH, 0); + if ((hca->attrs.device_cap_flags & IB_DEVICE_KNOWSEPOCH) == 0) + if_setflagbits(dev, IFF_NEEDSEPOCH, 0); infiniband_ifattach(priv->dev, NULL, priv->broadcastaddr);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202304171609.33HG9UOp003907>