On 3/30/2016 3:18 AM, John Fastabend wrote:
I would prefer to not have another strange quirk users have to
remember in order to do tx classification. So with this change
depending on the driver the queue selection precedence changes.
This change doesn't depend on the driver it affects all drivers that
implement the select queue ndo and use the default fallback
"pick_tx_queue" which this patch came to fix, or any driver that doesn't
implement the ndo (the fallback is the default in this case).
In short I agree with the problem statement but think we can find a
better solution. One idea that comes to mind is we can have a tc
action to force the queue selection? Now that we have the egress tc
hook it would probably be fairly cheap to implement and if users want
this behavior they can ask for it explicitly. If your thinking about
tc stuff we could fix the tooling to set this action when ever dcb is
turned on or hardware rate limiting is enabled, etc. And even if we
wanted we could have the driver add the rule in the cases where
firmware protocols are configuring the QOS/etc.
Why would you ask for a bug fix explicitly ? IMHO this how I expect the
pick _tx_queue routine to behave, why would I disable XPS in order for
select queue to choose according TC QoS ?
as this patch suggests we can benefit from both without any additional
tooling !
if (skb_vlan_tag_present(skb))
up = skb_vlan_tag_get(skb) >> VLAN_PRIO_SHIFT;
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index cb0d5d0..ad81ffe 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3130,16 +3130,16 @@ static inline int netif_set_xps_queue(struct net_device
*dev,
#endif
u16 __skb_tx_hash(const struct net_device *dev, struct sk_buff *skb,
- unsigned int num_tx_queues);
+ unsigned int num_tx_queues, int txq_hint);
[...]
And all this seems like it would only ever be called by drivers select
queue routines which I really wish we could kill off one of these days
instead of add to. Now if the signal is something higher in the stack
and not the driver I think it is OK.
I agree, drivers shouldn't call this function, the only reason drivers
call this function is to bypass get_xps_queue
and after this patch i don't think driver will need to call it, since it
will be called even if XPS is configured.
