[lkml]   [2022]   [Dec]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [PATCH] net/ncsi: Always use unicast source MAC address
On Fri, Dec 16, 2022 at 8:20 PM Peter Delevoryas <> wrote:
> > On Dec 16, 2022, at 10:29 AM, Alexander Duyck <> wrote:
> >
> > On Thu, Dec 15, 2022 at 5:08 PM Peter Delevoryas <> wrote:
> >>
> >>
> >>
> >>> On Dec 13, 2022, at 8:41 AM, Alexander H Duyck <> wrote:
> >>>
> >>> On Mon, 2022-12-12 at 16:47 -0800, Peter Delevoryas wrote:


> >
> >>> My main
> >>> concern would be that the dev_addr is not initialized for those first
> >>> few messages so you may be leaking information.
> >>>
> >>>> This might have the effect of causing the NIC to learn 2 MAC addresses from
> >>>> an NC-SI link if the BMC uses OEM Get MAC Address commands to change its
> >>>> initial MAC address, but it shouldn't really matter. Who knows if NIC's
> >>>> even have MAC learning enabled from the out-of-band BMC link, lol.
> >>>>
> >>>> [1]:
> >>>> [2]:
> >>>
> >>> The thing is the OpenBMC approach initializes the value themselves to
> >>> broadcast[3]. As a result the two code bases are essentially doing the
> >>> same thing since mac_addr is defaulted to the broadcast address when
> >>> the ncsi interface is registered.
> >>
> >> That’s a very good point, thanks for pointing that out, I hadn’t
> >> even noticed that!
> >>
> >> Anyways, let me know what you think of the traces I added above.
> >> Sorry for the delay, I’ve just been busy with some other stuff,
> >> but I do really actually care about upstreaming this (and several
> >> other NC-SI changes I’ll submit after this one, which are unrelated
> >> but more useful).
> >>
> >> Thanks,
> >> Peter
> >
> > So the NC-SI spec says any value can be used for the source MAC and
> > that broadcast "may" be used. I would say there are some debugging
> > advantages to using broadcast that will be obvious in a packet trace.
> Ehhhhh yeah I guess, but the ethertype is what I filter for. But sure,
> a broadcast source MAC is pretty unique too.
> > I wonder if we couldn't look at doing something like requiring
> > broadcast or LAA if the gma_flag isn't set.
> What is LAA? I’m out of the loop

Locally administered MAC address[4]. Basically it is a MAC address
that is generated locally such as your random MAC address. Assuming
the other end of the NC-SI link is using a MAC address with a vendor
OUI there should be no risk of collisions on a point-to-point link.
Essentially if you wanted to you could probably just generate a random
MAC address for the NCSI protocol and then use that in place of the
broadcast address.

> But also: aren’t we already using broadcast if the gma_flag isn’t set?
> - if (nca->ndp->gma_flag == 1)
> - memcpy(eh->h_source, nca->ndp->>dev_addr, ETH_ALEN);
> - else
> - eth_broadcast_addr(eh->h_source);
> + memcpy(eh->h_source, nca->ndp->>dev_addr, ETH_ALEN);

That I am not sure about. You were using this kernel without your
patch right? With your patch it would make sense to see that behavior,
but without I am not sure why you would see that address for any NC-SI
commands before the gma_flag is set.

> > With that we could at
> > least advertise that we don't expect this packet to be going out in a
> > real network as we cannot guarantee the MAC is unique.
> Yeah, but it probably wouldn’t help my simulation scenario.
> I guess it sounds like this patch is not a good idea, which to be fair,
> is totally reasonable.
> I can just add some iptables rules to tunnel these packets with a different
> source MAC, or fix the multicast socket issue I was having. It’s really
> not a big deal, and like you’re saying, we probably don’t want to make
> it harder to maintain _forever_.

Like I said before I would be good with either a Broadcast address OR
a LAA address. The one thing we need to watch out for though is any
sort of leak. One possible concern would be if for example you had 4
ports using 4 different MAC addresses but one BMC. You don't want to
accidently leak the MAC address from one port onto the other one. With
a LAA address if it were to leak and screw up ARP tables somewhere it
wouldn't be a big deal since it isn't expected to be switched in the
first place.

> I would just suggest praying for the next guy that tries to test NC-SI
> stuff with QEMU and finds out NC-SI traffic gets dropped by bridges.
> I had to resort to reading the source code and printing stuff with
> BPF to identify this. Maybe it’s more obvious to other people this wouldn’t
> work though.

Well it seems like NC-SI isn't meant to be bridged based on the fact
that it is using a broadcast MAC address as a source. If nothing else
I suppose you could try to work with the standards committee on that
to see what can be done to make the protocol more portable.. :-)


 \ /
  Last update: 2022-12-17 21:57    [W:0.092 / U:0.152 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site