diff mbox series

l2_packet_freebsd: Enable receiving priority tagged (VID=0) frames

Message ID EpfIknjTLAUEDcNyjpJZEILUN5Jda9z31C3I3GiSlH1Q9MkHyVs4NK0ideZfDv-3v1fQN5Eo0wXWwBVQbtZZpgTftY8092Agdb6mRJeMRTI=@rcm.sh
State Accepted
Headers show
Series l2_packet_freebsd: Enable receiving priority tagged (VID=0) frames | expand

Commit Message

R. Christian McDonald June 9, 2023, 4:27 p.m. UTC
Certain internet service providers transmit VLAN 0 priority tagged
EAPOL frames from the ONT towards the residential gateway. VID 0
should be ignored, and the frame processed according to the priority
set in the 802.1P bits and the encapsulated EtherType (i.e. EAPOL).

The pcap filter utilized by l2_packet_* is inadquate for this use case.

Here we modify the pcap filter on FreeBSD to accept both unencapsulated
and encapsulated (with VLAN 0) EAPOL EtherTypes. This preserves the
original filter behavior while also matching on encapsulated EAPOL.

Additional work is required to support this handling on other platforms.

We also modify the rx_receive handler to offset the packet buffer
and length when handling dot1q encapsualted frames so the existing
packet parsing code works as-is.

Finally, we bring along another patch from FreeBSD that handles the
case where the interface can disappear either by `ifconfig destroy`
or if a USB NIC is forcibly removed or experiences USB faults. This
patch uses `pcap_next_ex`, setting the packet pointer to NULL, and
terminating the event loop.

Signed-off-by: R. Christian McDonald <rcm@rcm.sh>
Sponsored by: Rubicon Communications, LLC ("Netgate")
---
 src/l2_packet/l2_packet_freebsd.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

Comments

Jouni Malinen Oct. 28, 2023, 3:04 p.m. UTC | #1
On Fri, Jun 09, 2023 at 04:27:12PM +0000, R. Christian McDonald wrote:
> Certain internet service providers transmit VLAN 0 priority tagged
> EAPOL frames from the ONT towards the residential gateway. VID 0
> should be ignored, and the frame processed according to the priority
> set in the 802.1P bits and the encapsulated EtherType (i.e. EAPOL).
> 
> The pcap filter utilized by l2_packet_* is inadquate for this use case.
> 
> Here we modify the pcap filter on FreeBSD to accept both unencapsulated
> and encapsulated (with VLAN 0) EAPOL EtherTypes. This preserves the
> original filter behavior while also matching on encapsulated EAPOL.
> 
> Additional work is required to support this handling on other platforms.
> 
> We also modify the rx_receive handler to offset the packet buffer
> and length when handling dot1q encapsualted frames so the existing
> packet parsing code works as-is.

I applied this part.

> Finally, we bring along another patch from FreeBSD that handles the
> case where the interface can disappear either by `ifconfig destroy`
> or if a USB NIC is forcibly removed or experiences USB faults. This
> patch uses `pcap_next_ex`, setting the packet pointer to NULL, and
> terminating the event loop.

But this is not really expected wpa_supplicant/hostapd behavior, so I
dropped this:

> diff --git a/src/l2_packet/l2_packet_freebsd.c b/src/l2_packet/l2_packet_freebsd.c
> @@ -82,7 +83,11 @@ static void l2_packet_receive(int sock, void *eloop_ctx, void *sock_ctx)
>  	unsigned char *buf;
>  	size_t len;
>  
> -	packet = pcap_next(pcap, &hdr);
> +	if (pcap_next_ex(pcap, &hdr, &packet) == -1) {
> +		wpa_printf(MSG_ERROR, "Error reading packet, has device disappeared?");
> +		packet = NULL;
> +		eloop_terminate();
> +	}

This seems to be a completely independent change and as such, should be
in its own patch, but calling eloop_terminate() does not feel correct
for this type of a case. A better approach would be to detect interface
removal in src/drivers/driver_*.c and report it to upper layers (as is
already done for nl80211).
diff mbox series

Patch

diff --git a/src/l2_packet/l2_packet_freebsd.c b/src/l2_packet/l2_packet_freebsd.c
index 60de9fe6b..f643a61a6 100644
--- a/src/l2_packet/l2_packet_freebsd.c
+++ b/src/l2_packet/l2_packet_freebsd.c
@@ -20,6 +20,7 @@ 
 #include <sys/sysctl.h>
 #endif /* __sun__ */
 
+#include <net/ethernet.h>
 #include <net/if.h>
 #include <net/if_dl.h>
 #include <net/route.h>
@@ -82,7 +83,11 @@  static void l2_packet_receive(int sock, void *eloop_ctx, void *sock_ctx)
 	unsigned char *buf;
 	size_t len;
 
-	packet = pcap_next(pcap, &hdr);
+	if (pcap_next_ex(pcap, &hdr, &packet) == -1) {
+		wpa_printf(MSG_ERROR, "Error reading packet, has device disappeared?");
+		packet = NULL;
+		eloop_terminate();
+	}
 
 	if (!l2->rx_callback || !packet || hdr.caplen < sizeof(*ethhdr))
 		return;
@@ -94,6 +99,11 @@  static void l2_packet_receive(int sock, void *eloop_ctx, void *sock_ctx)
 	} else {
 		buf = (unsigned char *) (ethhdr + 1);
 		len = hdr.caplen - sizeof(*ethhdr);
+		/* handle 8021Q encapsulated frames */
+		if (ethhdr->h_proto == htons(ETH_P_8021Q)) {
+			buf += ETHER_VLAN_ENCAP_LEN;
+			len -= ETHER_VLAN_ENCAP_LEN;
+		}
 	}
 	l2->rx_callback(l2->rx_callback_ctx, ethhdr->h_source, buf, len);
 }
@@ -122,10 +132,10 @@  static int l2_packet_init_libpcap(struct l2_packet_data *l2,
 	os_snprintf(pcap_filter, sizeof(pcap_filter),
 		    "not ether src " MACSTR " and "
 		    "( ether dst " MACSTR " or ether dst " MACSTR " ) and "
-		    "ether proto 0x%x",
+		    "( ether proto 0x%x or ( vlan 0 and ether proto 0x%x ) )",
 		    MAC2STR(l2->own_addr), /* do not receive own packets */
 		    MAC2STR(l2->own_addr), MAC2STR(pae_group_addr),
-		    protocol);
+		    protocol, protocol);
 	if (pcap_compile(l2->pcap, &pcap_fp, pcap_filter, 1, pcap_netp) < 0) {
 		fprintf(stderr, "pcap_compile: %s\n", pcap_geterr(l2->pcap));
 		return -1;