On Mon, Nov 15, 2021 at 06:09:28PM +0100, Toke Høiland-Jørgensen wrote:
Paolo Abeni [off-list ref] writes:
quoted
quoted
quoted
- pr_warn_once("%s XDP return value %u, expect packet loss!\n",
+ pr_warn_once("%s XDP return value %u on prog %d dev %s attach type %d, expect packet loss!\n",
act > act_max ? "Illegal" : "Driver unsupported",
- act);
+ act, prog->aux->id, dev->name, prog->expected_attach_type);
This will only ever trigger once per reboot even if the message differs,
right? Which makes it less useful as a debugging aid; so I'm not sure if
it's really worth it with this intrusive change unless we also do
something to add a proper debugging aid (like a tracepoint)...
Yes, the idea would be to add a tracepoint there, if there is general
agreement about this change.
I think this patch is needed because the WARN_ONCE splat gives
implicitly information about the related driver and attach type.
Replacing with a simple printk we lose them.
Ah, right, good point. Pointing that out in the commit message might be
a good idea; otherwise people may miss that ;)
Though it's quite a churn across the drivers I think extra verbosity here is justified.
I'd only suggest to print stable things. Like prog->aux->id probably has
little value for the person looking at the logs. That prog id is likely gone.
If it was prog->aux->name it would be more helpful.
Same with expected_attach_type. Why print it at all?
tracepoint is probably good idea too.