Re: [PATCH] net/packet: fix packet drop as of virtio gso

[Jianfeng Tan <jianfeng.tan@linux.alibaba.com>]

On 10/8/2018 11:14 AM, Jason Wang wrote:
>
>
> On 2018年09月29日 23:41, Jianfeng Tan wrote:
>> When we use raw socket as the vhost backend, a packet from virito with
>> gso offloading information, cannot be sent out in later validaton at
>> xmit path, as we did not set correct skb->protocol which is further used
>> for looking up the gso function.
>
> Hi:
>
> May I ask the reason for using raw socket for vhost? It was not a 
> common setup with little care in the past few years. And it was slow 
> since it lacks some recent improvements. Can it be replaced with e.g 
> macvtap?

Hi Jason,

Apologize for late response. We are in container environment, in which 
case veth is used mostly. Either tap or macvtap cannot be put into an 
isolated netns. Another thing could be macvlan as the backend of vhost, 
which is not supported either. So unfortunately, improving raw socket is 
the only choice I suppose.

Thanks,
Jianfeng


>
> Thanks
>
>>
>> To fix this, we set this field according to virito hdr information.
>>
>> Fixes: e858fae2b0b8f4 ("virtio_net: use common code for 
>> virtio_net_hdr and skb GSO conversion")
>>
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Jianfeng Tan <jianfeng.tan@linux.alibaba.com>
>> ---
>>   include/linux/virtio_net.h | 18 ++++++++++++++++++
>>   net/packet/af_packet.c     | 11 +++++++----
>>   2 files changed, 25 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
>> index 9397628a1967..cb462f9ab7dd 100644
>> --- a/include/linux/virtio_net.h
>> +++ b/include/linux/virtio_net.h
>> @@ -5,6 +5,24 @@
>>   #include <linux/if_vlan.h>
>>   #include <uapi/linux/virtio_net.h>
>>   +static inline int virtio_net_hdr_set_proto(struct sk_buff *skb,
>> +                       const struct virtio_net_hdr *hdr)
>> +{
>> +    switch (hdr->gso_type & ~VIRTIO_NET_HDR_GSO_ECN) {
>> +    case VIRTIO_NET_HDR_GSO_TCPV4:
>> +    case VIRTIO_NET_HDR_GSO_UDP:
>> +        skb->protocol = cpu_to_be16(ETH_P_IP);
>> +        break;
>> +    case VIRTIO_NET_HDR_GSO_TCPV6:
>> +        skb->protocol = cpu_to_be16(ETH_P_IPV6);
>> +        break;
>> +    default:
>> +        return -EINVAL;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>>   static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
>>                       const struct virtio_net_hdr *hdr,
>>                       bool little_endian)
>> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
>> index 75c92a87e7b2..d6e94dc7e290 100644
>> --- a/net/packet/af_packet.c
>> +++ b/net/packet/af_packet.c
>> @@ -2715,10 +2715,12 @@ static int tpacket_snd(struct packet_sock 
>> *po, struct msghdr *msg)
>>               }
>>           }
>>   -        if (po->has_vnet_hdr && virtio_net_hdr_to_skb(skb, vnet_hdr,
>> -                                  vio_le())) {
>> -            tp_len = -EINVAL;
>> -            goto tpacket_error;
>> +        if (po->has_vnet_hdr) {
>> +            if (virtio_net_hdr_to_skb(skb, vnet_hdr, vio_le())) {
>> +                tp_len = -EINVAL;
>> +                goto tpacket_error;
>> +            }
>> +            virtio_net_hdr_set_proto(skb, vnet_hdr);
>>           }
>>             skb->destructor = tpacket_destruct_skb;
>> @@ -2915,6 +2917,7 @@ static int packet_snd(struct socket *sock, 
>> struct msghdr *msg, size_t len)
>>           if (err)
>>               goto out_free;
>>           len += sizeof(vnet_hdr);
>> +        virtio_net_hdr_set_proto(skb, &vnet_hdr);
>>       }
>>         skb_probe_transport_header(skb, reserve);