Re: [PATCH] netconsole: allow selection of egress interface via MAC address

[Breno Leitao <leitao@debian.org>]

Hello Uday,

On Thu, Jan 09, 2025 at 07:43:44AM -0800, Breno Leitao wrote:
> On Wed, Jan 08, 2025 at 08:02:44AM -0700, Uday Shankar wrote:
> > On Fri, Jan 03, 2025 at 03:41:17AM -0800, Breno Leitao wrote:
> 
> > > This will change slightly local_mac meaning. At the same time, I am not
> > > sure local_mac is a very useful field as-is. The configuration might be
> > > a bit confusing using `local_mac` to define the target interface. I am
> > > wondering if creating a new field might be more appropriate. Maybe
> > > `dev_mac`? (I am not super confident this approach is better TBH, but, it
> > > seems easier to reason about).
> > 
> > Do you mean creating a new field called dev_mac which replaces
> > local_mac? I do agree that naming is a bit better but I'd be worried
> > about breaking programs which expect local_mac to exist. Having the
> > field go read-only --> read-write via this change feels a lot less
> > disruptive to preexisting programs than renaming the field.
> > 
> > Or do you mean creating a new field dev_mac which will live alongside
> > local_mac, and letting local_mac keep its existing semantics? It feels
> 
> Right, that is what I meant originally.
> 
> > like that would lead to messier code, since dev_mac's semantics are kind
> > of a superset of local_mac's semantics (e.g. after selecting and
> > enabling a netconsole via dev_name, local_mac is populated with the mac
> > address of the interface and we'd probably want the same for dev_mac as
> > well).
> > 
> > A third option would be dropping the configfs changes altogether, which
> > I'd be okay with - as I highlighted in the commit message, I suspect
> > this interface is far less likely to see real use than the command-line
> > parameter. 
> 
> I like this option better, in fact. I agree we don't need to expose it via
> configfs (at least for now), since configfs configuration solves a
> slightly different problem.
> 
> > A downside of this option though is that automated testing
> > becomes difficult, as we can't write a variant of netcons_basic.sh
> 
> True. I _think_ it is better to optimize for simplicity in such case,
> and skip the configfs changes (at least for now).
> 
> > without configfs support. We'd have to have a test which uses the
> > parameter directly, and I'm not sure if we have a testing framework for
> > the kernel which would support that.
>  
>  I am wondering if we can test it by turning netconsole into a module in
>  the test .config, and passing the netconsole parameters when loading
>  the module? 

I wanted to check in on the status of this patchset, as I haven't
received any updates in the past two weeks. I remain enthusiastic about
this feature and believe it would be a valuable addition to the next
major merge window.

If you need any assistance or support, please don't hesitate to reach
out. I'm more than happy to help.