pcap-linux: Obtain VLAN TPID from the kernel when available #346
Conversation
The VLAN TPID was hardcoded in pcap-linux.c but recent Linux kernel supports 802.1ad Q-in-Q. Since Linux 3.14, kernel gives the TPID value to the userspace when available.
Unify the VLAN TPID selection logic into a helper macro VLAN_TPID().
|
Looks good to me. |
| @@ -366,6 +366,12 @@ static void pcap_oneshot_mmap(u_char *user, const struct pcap_pkthdr *h, | |||
| const u_char *bytes); | |||
| #endif | |||
|
|
|||
| #ifdef TP_STATUS_VLAN_TPID_VALID | |||
| # define VLAN_TPID(hdr, hv) (((hv)->tp_vlan_tpid || ((hdr)->tp_status & TP_STATUS_VLAN_TPID_VALID)) ? (hv)->tp_vlan_tpid : ETH_P_8021Q) | |||
There was a problem hiding this comment.
Should this be as following?
# define VLAN_TPID(hdr, hv) ((((hv)->tp_vlan_tpid != 0) && ((hdr)->tp_status & TP_STATUS_VLAN_TPID_VALID)) ? (hv)->tp_vlan_tpid : ETH_P_8021Q)
There was a problem hiding this comment.
Because the tp_vlan_tpid is always defined as the unsigned (__u16) in TPACKET structures, that change is not needed, IMHO. And if the definition is changed to the signed in the future, I think this is a kernel's bug...
There was a problem hiding this comment.
Sorry for the noise, I was misreading your points.
The logic for the selecting TPID is same as the TCI, but should it be different logic only in case of the TPID? It seems pcap-linux uses the value when the kernel gives any value if TP_STATUS_*_VALID is defined.
There was a problem hiding this comment.
The reason I posted question was that second TP_STATUS_VLAN_TPID_VALID does nothing meaningful when conbined with series of past linux kernels. If a packet has tpid, then first condition will be met, because tpid won't be 0.
Anyway, vlan tag existence will be checked by vlan_tci, and not vlan_tpid. If TP_STATUS_VLAN_TPID_VALID is defined, then we need just check that bit without checking tp_vlan_tpid is 0 or not, IMHO.
There was a problem hiding this comment.
Yes, that's right. But then I think the macro just should be as following:
# define VLAN_TPID(hdr, hv) ((hdr)->tp_status & TP_STATUS_VLAN_TPID_VALID ? (hv)->tp_vlan_tpid : ETH_P_8021Q)
Of course the tp_vlan_tpid will be used in this case even if the TPID is 0, but it ought to be right in the userspace, because the kernel set the TP_STATUS_VLAN_TPID_VALID.
pcap-linux: Obtain VLAN TPID from the kernel when available
The VLAN TPID was hardcoded in pcap-linux.c but recent Linux kernel
supports 802.1ad Q-in-Q.
Since Linux 3.14, kernel gives the TPID value to the userspace when
available.