netdev - Re: [PATCH 09/15] net: hbl_en: add habanalabs Ethernet driver

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: <f45a71f9-640e-473a-9b80-90a50b087474@lunn.ch>
Date: Sun, 23 Jun 2024 16:46:00 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Omer Shpigelman <oshpigelman@...ana.ai>
Cc: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
	"ogabbay@...nel.org" <ogabbay@...nel.org>,
	Zvika Yehudai <zyehudai@...ana.ai>
Subject: Re: [PATCH 09/15] net: hbl_en: add habanalabs Ethernet driver

> > But what about when the system is under memory pressure? You say it
> > allocates memory. What happens if those allocations fail. Does
> > changing the MTU take me from a working system to a dead system? It is
> > good practice to not kill a working system under situations like
> > memory pressure. You try to first allocate the memory you need to
> > handle the new MTU, and only if successful do you free existing memory
> > you no longer need. That means if you cannot allocate the needed
> > memory, you still have the old memory, you can keep the old MTU and
> > return -ENOMEM, and the system keeps running.
> > 
> 
> That's a good optimization for these kind of on-the-fly configurations but
> as you wrote before, changing an MTU value is not a hot path so out of
> cost-benefit considerations we didn't find it mandatory to optimize this
> flow.

I would not call this an optimization. And it is not just about
changing the MTU. ethtool set_ringparam() is also likely to run into
this problem, and any other configuration which requires reallocating
the rings.

This is something else which comes up every few months on the list,
and drivers writers who monitor the list will write their drivers that
why, not 'optimise' it later.

> I get your point but still it will be good if it would be documented
> somewhere IMHO.

Kernel documentation is poor, agreed. But kernel policy is also
somewhat fluid, best practices change, and any developers can
influence that policy, different subsystems can and do have
contradictory policy, etc. The mailing list is the best place to learn
and to take part in this community. You need to be on the list for
other reasons as well.

> I'm familiar with this logic but I don't understand your point. The point
> you are making is that setting this Autoneg bit in lp_advertising is
> pointless? I see other vendors setting it too in case that autoneg was
> completed.
> Is that redundant also in their case? because it looks to me that in this
> case we followed the same logic and conventions other vendors followed.

Please show us the output from ethtool. Does it look like the example
i showed? I must admit, i'm more from the embedded world and don't
have access to high speed interfaces. But the basic concept of
auto-neg should not change that much.

	 Andrew