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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250203-capable-manipulative-angelfish-bebe71@leitao>
Date: Mon, 3 Feb 2025 05:12:37 -0800
From: Breno Leitao <leitao@...ian.org>
To: Uday Shankar <ushankar@...estorage.com>
Cc: "David S . Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Simon Horman <horms@...nel.org>, netdev@...r.kernel.org
Subject: Re: [PATCH] netconsole: allow selection of egress interface via MAC
 address

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.