Message-ID: <7f4a7177f249c3906343afb74ecef76bd0479723.camel@gmail.com>
Date: Fri, 01 Nov 2024 16:04:50 -0700
From: Alexander H Duyck <alexander.duyck@...il.com>
To: Andrew Lunn <andrew@...n.ch>, Mohsin Bashir <mohsin.bashr@...il.com>
Cc: netdev@...r.kernel.org, alexanderduyck@...com, kuba@...nel.org, 
 andrew+netdev@...n.ch, davem@...emloft.net, edumazet@...gle.com,
 pabeni@...hat.com,  kernel-team@...a.com, sanmanpradhan@...a.com,
 sdf@...ichev.me,  vadim.fedorenko@...ux.dev, hmohsin@...a.com
Subject: Re: [PATCH net-next v2] eth: fbnic: Add support to write TCE TCAM
 entries

On Fri, 2024-11-01 at 13:33 +0100, Andrew Lunn wrote:
> On Thu, Oct 31, 2024 at 03:13:40PM -0700, Mohsin Bashir wrote:
> > On 10/31/24 5:43 AM, Andrew Lunn wrote:
> > > On Wed, Oct 30, 2024 at 05:51:53PM -0700, Mohsin Bashir wrote:
> > > > Hi Andrew,
> > > > 
> > > 
> > > > Basically, in addition to the RX TCAM (RPC) that you mentioned, we
> > > > also have a TCAM on the TX path that enables traffic redirection for
> > > > BMC. Unlike other NICs where BMC diversion is typically handled by
> > > > firmware, FBNIC firmware does not touch anything host-related. In
> > > > this patch, we are writing MACDA entries from the RPC (Rx Parser and
> > > > Classifier) to the TX TCAM, allowing us to reroute any host traffic
> > > > destined for BMC.
> > > 
> > > Two TCAMs, that makes a bit more sense.
> > > 
> > > But why is this hooked into set_rx_mode? It is nothing to do with RX.
> > 
> > We are trying to maintain a single central function to handle MAC updates.
> 
> So you plan to call set_rx_mode() for any change to the TCAM, for any
> reason? When IGMP snooping asks you to add an multicast entry due to
> snooping?  ethtool --config-ntuple? Some TC actions? i assume you are
> going to call it yourself, not rely on the stack call set_rx_mode()
> for you?
> 

What the set_rx_mode function is doing is taking in all the MAC
addresses from all the various lists, unicast, multicast, BMC, and then
updating the TCAM as needed. We are keeping a copy of the TCAM in
software so if we call set_rx_mode with no change there is no change to
the hardware setup.

I think the TCE TCAM may be the only exception to that as it might
shift up or down as we are just parsing the list for BMC addresses and
writing them top down, or bottom up in order to preserve the entries in
the case of one being added or removed. So the table may shift if there
is no change, but the contents should stay the same.

Basically the BMC makes it so that we have to modify the ACT_TCAM rules
if we add/remove the BMC addresses, or if we toggle the ALL_MULTI flag
since the BMC can force us to have to place that rule at either entry
24 in the MACDA TCAM, or if it isn't present we place it at entry 31.

For the --config-ntuple we don't call it as the assumption is that we
aren't going to be modifying the multicast or unicast promiscuous flags
due to the rule addition. Instead that does a subset where it will use
fbnic_write_macda to update the MACDA TCAM if there is a new MAC
destination address added.

> > > I assume you have some mechanism to get the MAC address of the BMC. I
> > > would of thought you need to write one entry into the TCAM during
> > > probe, and you are done?
> > > 
> > > 	Andrew
> > 
> > Actually, we may need to write entries in other cases as well. The fact that
> > the BMC can come and go independently of the host would result in firmware
> > notifying the host of the resulting change. Consequently, the host would
> > need to make some changes that will be added in the following patch(es).
> 
> And the MAC address changes when the firmware goes away and comes back
> again? It is not burned into an OTP/EEPROM? What triggers
> set_rx_mode() being called in this condition?
> 
> 	Andrew

So there are BMC addresses that are passed in via the Firmware
Capabilities Response message. Unfortunately we have to deal with BMCs
that will bail on us and force us to remove the entries for it as soon
as they see the link go down and come back as soon as they see the link
come up. In addition the NCSI spec basically makes it so that we have
to support the BMC deciding to change the config or enable/disable the
channel on a whim.

In such a case we have to go through and take care of several items
including pointing our TCE TCAM at the BMC addresses, and then sending
a message to the firmware with the list of our unicast MAC addresses so
that it can send the BMC packets back to us if it is sending to our
unicast addresses. Unfortunately the BMC also tends to enjoy running in
multicast promiscuous so we don't have any way of knowing what
mutlicast entries it has so we normally will just have it send us all
it's multicast traffic and likewise all our multicast traffic will be
routed to it. It simplifies the handling a bit, but also is kind of
sloppy as far as I am concerned.

- Alex

Powered by blists - more mailing lists