Sorry, did not meant to add bms@ as reviewer.

While the comment was LLM-aided, the issue itself is indeed of relevance for both frr-based routing infra as well as for automation aspects of networking.

Reworked along the lines @bz asked for, and split into a stack so each piece can
be read on its own. This revision is now only the consumer -- the SIOCGI2C
handler -- and the tunables are gone.

Why did you abandon? Sorry for not reviewing in timely manner. From a quick look change doesn't look bad.

Sorry, I do not support FRR, and I do not use LLM driven tools in my development work, nor do I accept submissions which use them.

Hi Bruce,

@dsl @adrian — I took D58258#1368055 literally and built MACLINK, then ran it on
an LX2160A (SolidRun CEX7, UEFI/ACPI; dpni0 = dpmac.17 RGMII, dpni1 = dpmac.8 and
dpni2 = dpmac.9, both 10G SFP+ with modules in).

In D58258#1367937, @dsl wrote:

In D58258#1367599, @adrian wrote:

In D58258#1367549, @dsl wrote:

@adrian well, I'm not sure that the contributor actually understands the code. It seems AI/ML generated to me and isn't aligned with the idea of mine about maclink. I'm against the changes.

What do you mean by "maclink" ? They said they used AI assistance in developing it, but it seems mostly simple enough:

• sfp is an i2c device on a bus;
• the i2c bus doesn't HAVE to be hooked up to the MAC in any way; it in theory could be hanging off of some other i2c controller in the system;
• there's information about where said bus is linked to in FDT;
• some simple shenanigans are required to be able to talk to it and fetch configuration parameters.

What doesn't quite jive with your assumptions of stuff?

MACLINK is supposed to be an abstraction which hides possibly complex topology of the devices and their interconnections which constitute a multi-gigabit link between the MAC (which is a part of a SoC usually) and a PHY/SFP+ on a PCB. This is the best explanation I've to date: https://github.com/mcusim/freebsd-src/blob/dpaa2/sys/dev/maclink/maclink.c#L31. All of the existing MACLINK bits live in https://github.com/mcusim/freebsd-src/tree/dpaa2/sys/dev/maclink, but haven't been tested yet.

Personally, I'd like a proper abstraction to be introduced first with a clear understanding how a maclink_bus can be attached by the network interface drivers and introduce new (specific?) maclink adapters which will be incapsulating all of the complex logic to discover PHYs, SFF/SFPs, PCSs, etc. and let the maclink bus (and the attaching network interface) know about high-level events, e.g. state changes, link's up/down, etc.

In D58258#1367599, @adrian wrote:

In D58258#1367549, @dsl wrote:

@adrian well, I'm not sure that the contributor actually understands the code. It seems AI/ML generated to me and isn't aligned with the idea of mine about maclink. I'm against the changes.

What do you mean by "maclink" ? They said they used AI assistance in developing it, but it seems mostly simple enough:

• sfp is an i2c device on a bus;
• the i2c bus doesn't HAVE to be hooked up to the MAC in any way; it in theory could be hanging off of some other i2c controller in the system;
• there's information about where said bus is linked to in FDT;
• some simple shenanigans are required to be able to talk to it and fetch configuration parameters.

What doesn't quite jive with your assumptions of stuff?

In D58258#1367668, @yarshure_gmail.com wrote:

.. My reading is that you have in mind a MAC-link layer
for dpaa2 along the lines of Linux's phylink: one place that owns link state for a
DPMAC and drives it from whatever is attached - a PHY via MDIO, a fixed-link, or an
SFP cage - so that module presence/LOS, TX_DISABLE and rate selection are handled
there rather than in each consumer. If that is roughly it, please correct the
details and I will work to it.

I believe the ACPI path was never tested on main - or we haven't flipped the defaults yet -- but for sure it'll fail to link modules due to unresolved symbols (in the future).

@dsl - on maclink: I want to make sure I am answering the right objection, because
I cannot find maclink in the tree (only enum dpaa2_mac_link_type in dpaa2_mac.h),
so I am guessing at its shape. My reading is that you have in mind a MAC-link layer
for dpaa2 along the lines of Linux's phylink: one place that owns link state for a
DPMAC and drives it from whatever is attached - a PHY via MDIO, a fixed-link, or an
SFP cage - so that module presence/LOS, TX_DISABLE and rate selection are handled
there rather than in each consumer. If that is roughly it, please correct the
details and I will work to it.

v3 addresses @adrian's two points:

In D58258#1367549, @dsl wrote:

@adrian well, I'm not sure that the contributor actually understands the code. It seems AI/ML generated to me and isn't aligned with the idea of mine about maclink. I'm against the changes.

@adrian well, I'm not sure that the contributor actually understands the code. It seems AI/ML generated to me and isn't aligned with the idea of mine about maclink. I'm against the changes.

this looks fine; please just remove teh BSD copyright text itself as the SPDX + your copyright name/email is enough. Then we should be fine for landing it!

merged in 742c5498aca9a9a31a68eb9d5888edf36b7034dd

@pouria — you're right that this is solvable in the firewall, and I should have checked before offering the boot-window argument. One detail worth recording: on a stock system rcorder puts netif well before rc.d/ipfw, and net.inet.ip.fw.default_to_accept reads 1 on my hosts — so with ipfw loaded from rc.conf the window is real. It closes with ipfw_load="YES" and net.inet.ip.fw.default_to_accept=0 in loader.conf, which makes the default deny active before any interface is configured. That's the correct answer to my objection, and it belongs in documentation.

In D59160#1357307, @paulo_nlink.com.br wrote:

@pouria MLD's role is informing snooping switches and multicast routers where to forward group traffic so they can prune instead of flooding (RFC 4541). So "ND relies on MLD" holds on fabrics whose switches prune by snooping

@pouria — thanks for the reasoning chain, but I believe step 4 conflates two mechanisms. What limits NS processing to the right nodes on a flat L2 is the solicited-node-to-33:33:xx MAC mapping: NICs filter in hardware, no MLD involved. MLD's role is informing snooping switches and multicast routers where to forward group traffic so they can prune instead of flooding (RFC 4541). So "ND relies on MLD" holds on fabrics whose switches prune by snooping — and IXP peering LANs are the opposite case: they flood link-local multicast, which is why their operators can and do forbid MLD (IX.br PRT, Euro-IX). Your step 2 is actually the argument: exchanges permit ARP because IPv4 can't work without it, and they permit NS/NA while forbidding MLD because, on their fabric, ND works without it. That analysis was done by the people who run the switches.

I'm the original author of this code. I have run into exactly the same issue you have called out, but at COMEX/CME in Chicago with IGMPv3 being seen upstream of a demarc boundary where only PIM traffic was expected. This was on Linux, and we ended up filtering outgoing IGMP traffic with iptables; ~2010.

Thank you for your contribution.
I understand your concern.
I've encountered similar policies at multiple IXPs as well. (ofc not for MLD)

oh and I just hit this myself on a power8 box that /has/ bxe in it!

Superseded by D58721, which covers both fixes here on a rewritten common unwind path and adds the SR-IOV schema and core-offset refcount cases. Abandoning in favor of that.

I was independently dealing with similar issues while testing SR-IOV. Can you have a look at https://reviews.freebsd.org/D58721 and the linked fail(9) commit.

igc already does not panic per your own statement, so I'm not sure what you are arguing. My request is to unify lifecycle management as one commit. But now looking and iflib.c I think the igc_if_init should simply be dropped from resume and media_change so please do that instead.

@kbowling resume and media_change are not the same. They already inside a CTX_LOCK already, no panic.

It's one unit of work unifying a lifecycle issue

In D58629#1345834, @kbowling wrote:

Can you do the same for media_change and if_resume?

Can you do the same for media_change and if_resume?

Can you do the same for media_change and if_resume?