Thread (28 messages) 28 messages, 9 authors, 2023-06-16

Re: [PATCH net-next 1/3] iavf: add check for current MAC address in set_mac callback

From: Piotr Gardocki <hidden>
Date: 2023-06-07 10:29:49

On 06.06.2023 19:24, Jakub Kicinski wrote:
On Tue, 6 Jun 2023 12:21:07 +0200 Maciej Fijalkowski wrote:
quoted
quoted
quoted
couldn't this be checked the layer above then? and pulled out of drivers?
Probably it could, but I can't tell for all drivers if such request should
always be ignored. I'm not aware of all possible use cases for this callback
to be called and I can imagine designs where such request should be
always handled.  
if you can imagine a case where such request should be handled then i'm
all ears. it feels like this is in an optimization where everyone could
benefit from (no expert in this scope though), but yeah this callback went
into the wild and it's implemented all over the place.
+1, FWIW, this is a net-next change, let's try to put it in the core
unless we see a clear enough reason not to.
The reason I was not keen to move it to core is that a lot of drivers actually
perform some actions even when the MAC address doesn't change, like writing
to HW registers, performing link down, link up routine, notifying physical
function drivers, etc. I'm not aware if any of them really need this kind of logic,
so I can't stand strongly against new proposal. If the community is fine with it
we can move it to core code.

I need a piece of advice though:
1) Should I fix it in this patch set, or treat it as a separate thread?
2) I suppose the change is required only in dev_set_mac_address function, but
am I right assuming we should do it before call to dev_pre_changeaddr_notify
and return from function early? What about call to add_device_randomness?
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help