Bug 273559 - [iflib] numa allocation
Summary: [iflib] numa allocation
Status: New
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 13.2-STABLE
Hardware: amd64 Any
: --- Affects Only Me
Assignee: freebsd-net (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-09-04 10:15 UTC by Konrad
Modified: 2024-12-18 13:47 UTC (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Konrad 2023-09-04 10:15:46 UTC
Hello,

Based on https://reviews.freebsd.org/D19930 I did a patch for iflib:

diff --git a/sys/net/iflib.c b/sys/net/iflib.c
index d6dcb0931..4437f6e57 100644
--- a/sys/net/iflib.c
+++ b/sys/net/iflib.c
@@ -5953,7 +5953,7 @@ iflib_register(if_ctx_t ctx)
                        type = IFT_PPP;
        } else
                type = IFT_ETHER;
-       ifp = ctx->ifc_ifp = if_alloc(type);
+       ifp = ctx->ifc_ifp = if_alloc_dev(type, dev);
        if (ifp == NULL) {
                device_printf(dev, "can not allocate ifnet structure\n");
                return (ENOMEM);

for allocate NUMA local memory. I used it for lagg lacp egress port selection based on NUMA domain. So far everything worked fine (stable/13 build 12 Apr 2022). Yesterday I did upgrade to latest stable/13 and patch has stopped working. The same flow comes to ix1 (numa-domain 0) and goes out ix3 (numa-domain 1). Lagg0 has enabled use_numa (as before). System has a few vlans on lagg0 and does the routing

#dev numa domain
dev.ix.3.%domain: 1
dev.ix.2.%domain: 1
dev.ix.1.%domain: 0
dev.ix.0.%domain: 0

#interfaces
#10G - ix0
ifconfig_ix0="up"
#10G - ix1
ifconfig_ix1="up"
#10G - ix2
ifconfig_ix2="up"
#10G - ix3
ifconfig_ix3="up"

#LAGG LACP
ifconfig_lagg0="laggproto lacp laggport ix0 laggport ix1 laggport ix2 laggport ix3 up -wol -vlanhwtso -tso -lro"

lagg0: flags=8843<UP,BROADCAST,RUNNING,SIMPLEX,MULTICAST> metric 0 mtu 1500
	options=4e100bb<RXCSUM,TXCSUM,VLAN_MTU,VLAN_HWTAGGING,JUMBO_MTU,VLAN_HWCSUM,VLAN_HWFILTER,RXCSUM_IPV6,TXCSUM_IPV6,NOMAP>
	ether 90:e2:ba:78:9d:2c
	laggproto lacp lagghash l2,l3,l4
	lagg options:
		flags=15<USE_FLOWID,USE_NUMA,LACP_STRICT>
		flowid_shift: 16
	lagg statistics:
		active ports: 2
		flapping: 0
	lag id: [(8000,90-E2-BA-78-9D-2C,0152,0000,0000),
		 (0000,00-11-22-33-44-55,000C,0000,0000)]
	laggport: ix0 flags=0<> state=41<ACTIVITY,DEFAULTED>
		[(8000,90-E2-BA-78-9D-2C,8001,8000,0001),
		 (FFFF,00-00-00-00-00-00,0000,FFFF,0000)]
	laggport: ix1 flags=1c<ACTIVE,COLLECTING,DISTRIBUTING> state=3d<ACTIVITY,AGGREGATION,SYNC,COLLECTING,DISTRIBUTING>
		[(8000,90-E2-BA-78-9D-2C,0152,8000,0002),
		 (0000,00-11-22-33-44-55,000C,0000,03EF)]
	laggport: ix2 flags=0<> state=41<ACTIVITY,DEFAULTED>
		[(8000,90-E2-BA-78-9D-2C,8007,8000,0007),
		 (FFFF,00-00-00-00-00-00,0000,FFFF,0000)]
	laggport: ix3 flags=1c<ACTIVE,COLLECTING,DISTRIBUTING> state=3d<ACTIVITY,AGGREGATION,SYNC,COLLECTING,DISTRIBUTING>
		[(8000,90-E2-BA-78-9D-2C,0152,8000,0008),
		 (0000,00-11-22-33-44-55,000C,0000,03EF)]
	groups: lagg
	media: Ethernet autoselect
	status: active
	nd6 options=29<PERFORMNUD,IFDISABLED,AUTO_LINKLOCAL>



Where is the problem ?
Comment 1 Zhenlei Huang freebsd_committer freebsd_triage 2023-09-07 07:12:11 UTC
(In reply to Konrad from comment #0)

The lagg(4) has no changes related to NUMA since Nov 19 2021.

From the logic in `sys/net/if_lagg.c`: 
```
struct lagg_port *
lacp_select_tx_port(struct lagg_softc *sc, struct mbuf *m, int *err)
{
        struct lacp_softc *lsc = LACP_SOFTC(sc);
        uint32_t hash;
        uint8_t numa_domain;

        if ((sc->sc_opts & LAGG_OPT_USE_FLOWID) &&
            M_HASHTYPE_GET(m) != M_HASHTYPE_NONE)
                hash = m->m_pkthdr.flowid >> sc->flowid_shift;
        else
                hash = m_ether_tcpip_hash(sc->sc_flags, m, lsc->lsc_hashkey);

        numa_domain = m->m_pkthdr.numa_domain;
        return (lacp_select_tx_port_by_hash(sc, hash, numa_domain, err));
}
```

I guess `numa_domain` in mbuf packet header is not set correctly. That should be done in driver IIUC.

I do not have NUMA devices, can you please share the debug info for `m->m_pkthdr.numa_domain` ?
Comment 2 commit-hook freebsd_committer freebsd_triage 2024-12-04 18:35:28 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=3d642b0f71c501dd9ee7aa0487788f619900d297

commit 3d642b0f71c501dd9ee7aa0487788f619900d297
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2024-12-04 01:13:02 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2024-12-04 16:22:50 +0000

    iflib: Set the NUMA domain in receive packet headers

    Reading PR 273559 made me realize that commit 767723ddebe9 is
    incomplete.  iflib should set the NUMA domain of received packets before
    passing them to protocol layers.

    PR:             273559
    Reviewed by:    zlei, kbowling, erj
    Fixes:          767723ddebe9 ("iflib: Use if_alloc_dev() to allocate the ifnet")
    MFC after:      2 weeks
    Differential Revision:  https://reviews.freebsd.org/D47841

 sys/net/iflib.c | 3 +++
 1 file changed, 3 insertions(+)
Comment 3 commit-hook freebsd_committer freebsd_triage 2024-12-18 13:47:25 UTC
A commit in branch stable/14 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=113875f91607f385e87a54e017ac61f842df3282

commit 113875f91607f385e87a54e017ac61f842df3282
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2024-12-04 01:13:02 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2024-12-18 13:43:49 +0000

    iflib: Set the NUMA domain in receive packet headers

    Reading PR 273559 made me realize that commit 767723ddebe9 is
    incomplete.  iflib should set the NUMA domain of received packets before
    passing them to protocol layers.

    PR:             273559
    Reviewed by:    zlei, kbowling, erj
    Fixes:          767723ddebe9 ("iflib: Use if_alloc_dev() to allocate the ifnet")
    MFC after:      2 weeks
    Differential Revision:  https://reviews.freebsd.org/D47841

    (cherry picked from commit 3d642b0f71c501dd9ee7aa0487788f619900d297)

 sys/net/iflib.c | 3 +++
 1 file changed, 3 insertions(+)