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

Hi Daniel

quoted

quoted

quoted

+                       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.