[ovs-dev] [PATCH] netdev_afxdp: Detects combined channels and aborts wrong config
u9012063 at gmail.com
Thu Jan 16 19:21:05 UTC 2020
Thanks for the feedback.
The original problem we faced is that when using AF_XDP on physical device,
users often forgets to setup combined channel (ethool -L eth0 combined N).
Assume the device's default combined channel is 8, and ovs creates only 1 n_rxq,
(ovs-vsctl -- set int eth0 n_rxq=1 ...). Then there is no warning
message shows up,
and device attached to OVS successfully but receives no packet. Because XDP
program is attached to all 8 queues, and only 1 queues has XDP socket
packets. So this patch adds some warnings.
> > Does this command return maximum possible number of channels or
> > number of currently enabled channels?
> Thanks for review. This command can return both the maximum possible
> number of channels and the currently enabled channels.
> > In first case we'll end up with a need to configure huge number of queues.
> > In second case it's not guaranteed that number of channels will not be
> > changed later. (Does enabling more channels generates if-notifier event?)
> Yes, it generates if-notifier event.
I think when combined channel changed, the device goes through some reset
process, ex: to drain the queues and re-assign queue id.
> > Another point is why we need a configurable parameter that is allowed to
> > be configured with a single value only?
I don't understand.
> > And there is a possible usecase for not having all the queues attached
> > to OVS. For example, if you have a custom xdp program loaded, you may
> > redirect specific traffic to fast afxdp queues and other traffic to
> > kernel or system datapath. This might be useful for quick handling
> > of undesired or malicious traffic.
Yes, that's an interesting use case! This will require loading custom
instead of the built-in one in libbpf.
I will send out new version of the custom XDP program patch.
> > However, this still make sense to limit the maximum number of queues
> > with the number of actually available channels instead of having a
> > hardcoded constant (MAX_XSKQ).
> I agree that it is a valid use case to attach some queues to OVS and
> attach other XDP programs to the other queues. In this case, it is
> not appropriate to fail the afxdp setup when n_rxq != # or combined
> channels or # of rx channels.
I think we should fail, or at least log error message.
When n_rxq != # combined channels, OVS attaches XDP programs to all
combined channels, but only handle n_rxq queues. Packets will be dropped
at the rest queues. Remember right now, the XDP program attachment is
per-netdev to all queues, and there is no per-queue XDP program support.
> On the other hand, does it make sense to log the current # of combined
> channels and the current # of rx channels in the INFO level? It could
> be helpful to detect configuration issue on both use cases (attach all
> all the queues to OVS or not).
I would live to at least log the n_rxq and combined channels.
More information about the dev