Re: [PATCH net-next] net: enable RPS on vlan devices

[Shannon Nelson <shannon.nelson@oracle.com>]

On 10/9/2018 6:04 PM, Eric Dumazet wrote:
> 
> On 10/09/2018 05:41 PM, Shannon Nelson wrote:
>> From: Silviu Smarandache <silviu.smarandache@oracle.com>
>>
>> This patch modifies the RPS processing code so that it searches
>> for a matching vlan interface on the packet and then uses the
>> RPS settings of the vlan interface.  If no vlan interface
>> is found or the vlan interface does not have RPS enabled,
>> it will fall back to the RPS settings of the underlying device.
>>
>> In supporting VMs where we can't control the OS being used,
>> we'd like to separate the VM cpu processing from the host's
>> cpus as a way to help mitigate the impact of the L1TF issue.
>> When running the VM's traffic on a vlan we can stick the Rx
>> processing on one set of cpus separate from the VM's cpus.
>> Yes, choosing to use this may cause a bit of throughput pain
>> when the packets are actually passed into the VM and have to
>> move from one cache to another.
>>
>> Orabug: 28645929
>>
>> Signed-off-by: Silviu Smarandache <silviu.smarandache@oracle.com>
>> Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
>> ---
>>   net/core/dev.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++---
>>   1 file changed, 56 insertions(+), 3 deletions(-)
>>
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index 0b2d777..1da3f63 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -3971,8 +3971,8 @@ set_rps_cpu(struct net_device *dev, struct sk_buff *skb,
>>    * CPU from the RPS map of the receiving queue for a given skb.
>>    * rcu_read_lock must be held on entry.
>>    */
>> -static int get_rps_cpu(struct net_device *dev, struct sk_buff *skb,
>> -		       struct rps_dev_flow **rflowp)
>> +static int __get_rps_cpu(struct net_device *dev, struct sk_buff *skb,
>> +			 struct rps_dev_flow **rflowp)
>>   {
>>   	const struct rps_sock_flow_table *sock_flow_table;
>>   	struct netdev_rx_queue *rxqueue = dev->_rx;
>> @@ -4066,6 +4066,35 @@ static int get_rps_cpu(struct net_device *dev, struct sk_buff *skb,
>>   	return cpu;
>>   }
>>   
>> +static int get_rps_cpu(struct net_device *dev, struct sk_buff *skb,
>> +		       struct rps_dev_flow **rflowp)
>> +{
>> +	/* Check for a vlan device with RPS settings */
>> +	if (skb_vlan_tag_present(skb)) {
>> +		struct net_device *vdev;
>> +		u16 vid;
>> +
>> +		vid = skb_vlan_tag_get_id(skb);
>> +		vdev = __vlan_find_dev_deep_rcu(dev, skb->vlan_proto, vid);
>> +		if (vdev) {
>> +			/* recorded queue is not referring to the vlan device.
>> +			 * Save and restore it
>> +			 */
>> +			int cpu;
>> +			u16 queue_mapping = skb_get_queue_mapping(skb);
>> +
>> +			skb_set_queue_mapping(skb, 0);
>> +			cpu = __get_rps_cpu(vdev, skb, rflowp);
>> +			skb_set_queue_mapping(skb, queue_mapping);
> 
> This is really ugly :/

Hence the reason we sent this as an RFC a couple of weeks ago.  We got 
no response, so followed up with this patch in order to get some input. 
Do you have any suggestions for how we might accomplish this in a less 
ugly way?

> 
> Also what makes vlan so special compared to say macvlan ?

Only that vlan was the itch that Silviu needed to scratch.  If we can 
solve this for vlan, then perhaps we'll have a template to follow for 
other upper devices.

sln