Re: [PATCH net-next] packet mmap : allow the user to choose tx data offset.

[pchavent <Paul.Chavent@onera.fr>]

Hi Daniel

>>>> +                       switch (po->tp_version) {
>>>> +                       case TPACKET_V2:
>>>> +                               off = ph.h2->tp_mac;
>>>> +                               break;
>>>> +                       default:
>>>> +                               off = ph.h1->tp_mac;
>>>
>>>
>>> TPACKET_V1 as default is wrong since there's also TPACKET_V3. What
>>> about TPACKET_V3 in general in your patch? You simply ignore it.
>>
>>
>> I have reproduced the pattern found some lines above (1868-1875)
>>
>> switch (po->tp_version) {
>> case TPACKET_V2:
>>         tp_len = ph.h2->tp_len;
>>         break;
>> default:
>>         tp_len = ph.h1->tp_len;
>>         break;
>> }
>>
>> Do you suggest me to add a case TPACKET_V3 and get tp_len from 
>> ph.h3->tp_len
>> ?
>
> If I'm not mistaken, with your current patch you can do the
> setsockopt(2) call on a tpacket v3 socket and then default to a
> tpacket v1 in your switch statement. This doesn't seem right. Thus,
> this should better be fixed in whatever way.

It seems that "Opening a Tx-ring is NOT supported in TPACKET_V3" (see 
comment in packet_set_ring). So no one could be able to use the TX_OFF 
feature with a TPACKET_V3.

The tpacket_snd function call some subroutines that call WARN and BUG 
(like in __packet_set_status),
and other ones that simply ignore the tpacket v3 case (like in 
tpacket_fill_skb).

What do you suggest ?

I add a case that call WARN and BUG like in {__packet_set_status, 
__packet_get_status, packet_increment_rx_head} where we get tp_len in 
the tpacket_fill_skb function ?

Or i let the code as is ?

Regards.

Paul.