* packet_mmap, 802.1Q and bridges @ 2013-07-29 16:53 Phil Sutter 2013-07-31 17:27 ` Parse ethernet headers in af_packet.c Phil Sutter 0 siblings, 1 reply; 14+ messages in thread From: Phil Sutter @ 2013-07-29 16:53 UTC (permalink / raw) To: netdev; +Cc: Ben Greear, Stephen Hemminger, bridge, David S. Miller Hello everyone, I have found a situation where sending of VLAN tagged packets via packet_mmap over a bridge creates MTU problems. The setup is as follows: - physical interface eth0 (mv643xx) - bridge interface br0 contains just eth0 - packet_mmap sender bound to br0, socket() called with SOCK_RAW and ETH_P_ALL as arguments, PACKET_VERSION set to TPACKET_V2 - MTU is 1500 on all involved interfaces When userspace sends a full frame, it will pass tpacket_snd which will reject it unless there is a small patch of mine applied (for patch and related discussion, see: http://patchwork.ozlabs.org/patch/67422/). Tpacket_snd creates an SKB around the passed data, with 'protocol' field set to whatever was specified at socket() stage (in this case, ETH_P_ALL). Eventually, dev_queue_xmit() is called. The SKB created in af_packet.c is passed to the bridge code, while the interesting part is the function br_dev_queue_push_xmit() in net/bridge/br_forward.c. This function checks the frame's length against the device's MTU, therefore calling packet_length() which in turn does look at the SKB's protocol field for whether to account for an additional VLAN header or not. In this case, br_dev_queue_push_xmit() will drop the frame since ETH_P_ALL != ETH_P_8021Q. Using tcpdump, I could observe the packet occurs on br0, but not eth0. Changing packet_length() to also subtract VLAN_HLEN if skb->protocol is htons(ETH_P_ALL) fixed the problem. While preparing this email, I read again the discussion referenced above and found it surprisingly adequate for the problem described here. If I am not mistaken, the consensus was whether we should parse the given buffer in order to extract the real ethernet protocol or just account for four extra bytes, regardless of the actual packet. IMHO the problem described above is a strong argument for the first option. There is code which depends on skb->protocol's content to be correct, so af_packet should stick to it. Comments are highly appreciated. In the meantime, I will have a look at how to parse the packet_mmap buffer. If you got anything in that direction already, please feel free to come forward. Best wishes, Phil ^ permalink raw reply [flat|nested] 14+ messages in thread
* Parse ethernet headers in af_packet.c 2013-07-29 16:53 packet_mmap, 802.1Q and bridges Phil Sutter @ 2013-07-31 17:27 ` Phil Sutter 2013-07-31 17:27 ` [PATCH 1/3] af_packet: when sending ethernet frames, parse header for skb->protocol Phil Sutter 0 siblings, 1 reply; 14+ messages in thread From: Phil Sutter @ 2013-07-31 17:27 UTC (permalink / raw) To: David S. Miller; +Cc: netdev Hi, This is an AF_PACKET specific solution to the problems discussed in [1] and [2]. Basically this incorporates calling eth_type_trans() for packets to send when the outgoing device's type is ARPHRD_ETHER. In order to having a more generic solution which could be used in other places (like e.g. tun.c), I second Michael's approach in [2], but would extend it to a generic equivalent to eth_type_trans(). A device-type specific layer-2 header parser if you will. Note that eth_type_trans() also sets skbuff's dev, pkt_type and mac_header fields. A dev->header_ops->type_trans() could do just that plus set skb->protocol (maybe only if it's ETH_P_ALL). [1] http://patchwork.ozlabs.org/patch/67422/ [2] http://lists.openwall.net/netdev/2010/01/06/38 Best wishes, Phil ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/3] af_packet: when sending ethernet frames, parse header for skb->protocol 2013-07-31 17:27 ` Parse ethernet headers in af_packet.c Phil Sutter @ 2013-07-31 17:27 ` Phil Sutter 2013-07-31 17:27 ` [PATCH 2/3] af_packet: fix for sending VLAN frames via packet_mmap Phil Sutter 2013-08-01 0:30 ` [PATCH 1/3] af_packet: when sending ethernet frames, parse header for skb->protocol David Miller 0 siblings, 2 replies; 14+ messages in thread From: Phil Sutter @ 2013-07-31 17:27 UTC (permalink / raw) To: David S. Miller; +Cc: netdev This may be necessary when the SKB is passed to other layers on the go, which may check the protocol field on their own. An example is a VLAN packet sent using AF_PACKET out on a bridge interface. The bridging code does check the SKB size, accounting for any VLAN header only if the protocol field is set accordingly. Signed-off-by: Phil Sutter <phil@nwl.cc> --- net/packet/af_packet.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c index 20a1bd0..e549e17 100644 --- a/net/packet/af_packet.c +++ b/net/packet/af_packet.c @@ -88,6 +88,7 @@ #include <linux/virtio_net.h> #include <linux/errqueue.h> #include <linux/net_tstamp.h> +#include <linux/if_arp.h> #ifdef CONFIG_INET #include <net/inet_common.h> @@ -2005,6 +2006,9 @@ static int tpacket_fill_skb(struct packet_sock *po, struct sk_buff *skb, if (unlikely(err)) return err; + if (dev->type == ARPHRD_ETHER) + skb->protocol = eth_type_trans(skb, dev); + data += dev->hard_header_len; to_write -= dev->hard_header_len; } @@ -2324,6 +2328,13 @@ static int packet_snd(struct socket *sock, sock_tx_timestamp(sk, &skb_shinfo(skb)->tx_flags); + if (dev->type == ARPHRD_ETHER) { + skb->protocol = eth_type_trans(skb, dev); + } else { + skb->protocol = proto; + skb->dev = dev; + } + if (!gso_type && (len > dev->mtu + reserve + extra_len)) { /* Earlier code assumed this would be a VLAN pkt, * double-check this now that we have the actual @@ -2338,8 +2349,6 @@ static int packet_snd(struct socket *sock, } } - skb->protocol = proto; - skb->dev = dev; skb->priority = sk->sk_priority; skb->mark = sk->sk_mark; -- 1.8.1.5 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/3] af_packet: fix for sending VLAN frames via packet_mmap 2013-07-31 17:27 ` [PATCH 1/3] af_packet: when sending ethernet frames, parse header for skb->protocol Phil Sutter @ 2013-07-31 17:27 ` Phil Sutter 2013-07-31 17:27 ` [PATCH 3/3] af_packet: simplify VLAN frame check in packet_snd Phil Sutter 2013-08-01 0:30 ` [PATCH 1/3] af_packet: when sending ethernet frames, parse header for skb->protocol David Miller 1 sibling, 1 reply; 14+ messages in thread From: Phil Sutter @ 2013-07-31 17:27 UTC (permalink / raw) To: David S. Miller; +Cc: netdev Since tpacket_fill_skb() parses the protocol field in ethernet frames' headers, it's easy to see if any passed frame is a VLAN one and account for the extended size. But as the real protocol does not turn up before tpacket_fill_skb() runs which in turn also checks the frame length, move the max frame length calculation into the function. Signed-off-by: Phil Sutter <phil@nwl.cc> --- net/packet/af_packet.c | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c index e549e17..ce7b4f0 100644 --- a/net/packet/af_packet.c +++ b/net/packet/af_packet.c @@ -1924,7 +1924,7 @@ static int tpacket_fill_skb(struct packet_sock *po, struct sk_buff *skb, __be16 proto, unsigned char *addr, int hlen) { union tpacket_uhdr ph; - int to_write, offset, len, tp_len, nr_frags, len_max; + int to_write, offset, len, tp_len, nr_frags, len_max, max_frame_len; struct socket *sock = po->sk.sk_socket; struct page *page; void *data; @@ -1947,10 +1947,6 @@ static int tpacket_fill_skb(struct packet_sock *po, struct sk_buff *skb, tp_len = ph.h1->tp_len; break; } - if (unlikely(tp_len > size_max)) { - pr_err("packet size is too long (%d > %d)\n", tp_len, size_max); - return -EMSGSIZE; - } skb_reserve(skb, hlen); skb_reset_network_header(skb); @@ -2013,6 +2009,18 @@ static int tpacket_fill_skb(struct packet_sock *po, struct sk_buff *skb, to_write -= dev->hard_header_len; } + max_frame_len = dev->mtu + dev->hard_header_len; + if (skb->protocol == htons(ETH_P_8021Q)) + max_frame_len += VLAN_HLEN; + + if (size_max > max_frame_len) + size_max = max_frame_len; + + if (unlikely(tp_len > size_max)) { + pr_err("packet size is too long (%d > %d)\n", tp_len, size_max); + return -EMSGSIZE; + } + offset = offset_in_page(data); len_max = PAGE_SIZE - offset; len = ((to_write > len_max) ? len_max : to_write); @@ -2051,7 +2059,7 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg) struct net_device *dev; __be16 proto; bool need_rls_dev = false; - int err, reserve = 0; + int err; void *ph; struct sockaddr_ll *saddr = (struct sockaddr_ll *)msg->msg_name; int tp_len, size_max; @@ -2084,8 +2092,6 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg) if (unlikely(dev == NULL)) goto out; - reserve = dev->hard_header_len; - err = -ENETDOWN; if (unlikely(!(dev->flags & IFF_UP))) goto out_put; @@ -2093,9 +2099,6 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg) size_max = po->tx_ring.frame_size - (po->tp_hdrlen - sizeof(struct sockaddr_ll)); - if (size_max > dev->mtu + reserve) - size_max = dev->mtu + reserve; - do { ph = packet_current_frame(po, &po->tx_ring, TP_STATUS_SEND_REQUEST); -- 1.8.1.5 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 3/3] af_packet: simplify VLAN frame check in packet_snd 2013-07-31 17:27 ` [PATCH 2/3] af_packet: fix for sending VLAN frames via packet_mmap Phil Sutter @ 2013-07-31 17:27 ` Phil Sutter 0 siblings, 0 replies; 14+ messages in thread From: Phil Sutter @ 2013-07-31 17:27 UTC (permalink / raw) To: David S. Miller; +Cc: netdev For ethernet frames, eth_type_trans() already parses the header, so one can skip this when checking the frame size. Signed-off-by: Phil Sutter <phil@nwl.cc> --- net/packet/af_packet.c | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c index ce7b4f0..bbe1ece 100644 --- a/net/packet/af_packet.c +++ b/net/packet/af_packet.c @@ -2333,23 +2333,16 @@ static int packet_snd(struct socket *sock, if (dev->type == ARPHRD_ETHER) { skb->protocol = eth_type_trans(skb, dev); + if (skb->protocol == htons(ETH_P_8021Q)) + reserve += VLAN_HLEN; } else { skb->protocol = proto; skb->dev = dev; } if (!gso_type && (len > dev->mtu + reserve + extra_len)) { - /* Earlier code assumed this would be a VLAN pkt, - * double-check this now that we have the actual - * packet in hand. - */ - struct ethhdr *ehdr; - skb_reset_mac_header(skb); - ehdr = eth_hdr(skb); - if (ehdr->h_proto != htons(ETH_P_8021Q)) { - err = -EMSGSIZE; - goto out_free; - } + err = -EMSGSIZE; + goto out_free; } skb->priority = sk->sk_priority; -- 1.8.1.5 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] af_packet: when sending ethernet frames, parse header for skb->protocol 2013-07-31 17:27 ` [PATCH 1/3] af_packet: when sending ethernet frames, parse header for skb->protocol Phil Sutter 2013-07-31 17:27 ` [PATCH 2/3] af_packet: fix for sending VLAN frames via packet_mmap Phil Sutter @ 2013-08-01 0:30 ` David Miller 2013-08-01 10:35 ` Phil Sutter 1 sibling, 1 reply; 14+ messages in thread From: David Miller @ 2013-08-01 0:30 UTC (permalink / raw) To: phil; +Cc: netdev From: Phil Sutter <phil@nwl.cc> Date: Wed, 31 Jul 2013 19:27:45 +0200 > @@ -2324,6 +2328,13 @@ static int packet_snd(struct socket *sock, > > sock_tx_timestamp(sk, &skb_shinfo(skb)->tx_flags); > > + if (dev->type == ARPHRD_ETHER) { > + skb->protocol = eth_type_trans(skb, dev); > + } else { > + skb->protocol = proto; > + skb->dev = dev; > + } > + > if (!gso_type && (len > dev->mtu + reserve + extra_len)) { > /* Earlier code assumed this would be a VLAN pkt, > * double-check this now that we have the actual > @@ -2338,8 +2349,6 @@ static int packet_snd(struct socket *sock, > } > } > > - skb->protocol = proto; > - skb->dev = dev; > skb->priority = sk->sk_priority; > skb->mark = sk->sk_mark; I don't see anything explaining why you are avoiding setting skb->dev when you use eth_type_trans() to set the skb->protocol field. Also, why isn't the user setting the protocol field correctly? Isn't that the _real_ source of these problems? This patch series cannot be applied as is, it's either wrong, or explanations are missing from the commit messages. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] af_packet: when sending ethernet frames, parse header for skb->protocol 2013-08-01 0:30 ` [PATCH 1/3] af_packet: when sending ethernet frames, parse header for skb->protocol David Miller @ 2013-08-01 10:35 ` Phil Sutter 2013-08-02 1:12 ` David Miller 0 siblings, 1 reply; 14+ messages in thread From: Phil Sutter @ 2013-08-01 10:35 UTC (permalink / raw) To: David Miller; +Cc: netdev Hi, On Wed, Jul 31, 2013 at 05:30:17PM -0700, David Miller wrote: > From: Phil Sutter <phil@nwl.cc> > Date: Wed, 31 Jul 2013 19:27:45 +0200 > > > @@ -2324,6 +2328,13 @@ static int packet_snd(struct socket *sock, > > > > sock_tx_timestamp(sk, &skb_shinfo(skb)->tx_flags); > > > > + if (dev->type == ARPHRD_ETHER) { > > + skb->protocol = eth_type_trans(skb, dev); > > + } else { > > + skb->protocol = proto; > > + skb->dev = dev; > > + } > > + > > if (!gso_type && (len > dev->mtu + reserve + extra_len)) { > > /* Earlier code assumed this would be a VLAN pkt, > > * double-check this now that we have the actual > > @@ -2338,8 +2349,6 @@ static int packet_snd(struct socket *sock, > > } > > } > > > > - skb->protocol = proto; > > - skb->dev = dev; > > skb->priority = sk->sk_priority; > > skb->mark = sk->sk_mark; > > I don't see anything explaining why you are avoiding setting skb->dev when > you use eth_type_trans() to set the skb->protocol field. Simply because eth_type_trans() sets skb->dev already. You're right, I probably should have mentioned this explicitly. At least my introductory mail elaborated on this. > Also, why isn't the user setting the protocol field correctly? Isn't > that the _real_ source of these problems? Because the user specifies the value later used for skb->protocol at either socket(), bind() or sendto() (i.e. flush) stage. Having both "regular" and VLAN frames in the same TX ring is not possible at least. > This patch series cannot be applied as is, it's either wrong, or > explanations are missing from the commit messages. Hmm. Is it OK to rely on eth_type_trans() setting skb->dev after all? Depending on that, I would either add a comment or drop the change. Best wishes, Phil ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] af_packet: when sending ethernet frames, parse header for skb->protocol 2013-08-01 10:35 ` Phil Sutter @ 2013-08-02 1:12 ` David Miller 2013-08-02 9:37 ` [PATCH v2 " Phil Sutter 0 siblings, 1 reply; 14+ messages in thread From: David Miller @ 2013-08-02 1:12 UTC (permalink / raw) To: phil; +Cc: netdev From: Phil Sutter <phil@nwl.cc> Date: Thu, 1 Aug 2013 12:35:09 +0200 > Is it OK to rely on eth_type_trans() setting skb->dev after all? Yes, it is. ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 1/3] af_packet: when sending ethernet frames, parse header for skb->protocol 2013-08-02 1:12 ` David Miller @ 2013-08-02 9:37 ` Phil Sutter 2013-08-02 9:37 ` [PATCH v2 2/3] af_packet: fix for sending VLAN frames via packet_mmap Phil Sutter 2013-08-02 21:58 ` [PATCH v2 1/3] af_packet: when sending ethernet frames, parse header for skb->protocol David Miller 0 siblings, 2 replies; 14+ messages in thread From: Phil Sutter @ 2013-08-02 9:37 UTC (permalink / raw) To: David S. Miller; +Cc: netdev This may be necessary when the SKB is passed to other layers on the go, which check the protocol field on their own. An example is a VLAN packet sent out using AF_PACKET on a bridge interface. The bridging code checks the SKB size, accounting for any VLAN header only if the protocol field is set accordingly. Note that eth_type_trans() sets skb->dev to the passed argument, so this can be skipped in packet_snd() for ethernet frames, as well. Signed-off-by: Phil Sutter <phil@nwl.cc> --- Changes since V1: - minor lingual review of the comment - added note about setting skb->dev for non-ethernet frames only --- net/packet/af_packet.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c index 20a1bd0..e549e17 100644 --- a/net/packet/af_packet.c +++ b/net/packet/af_packet.c @@ -88,6 +88,7 @@ #include <linux/virtio_net.h> #include <linux/errqueue.h> #include <linux/net_tstamp.h> +#include <linux/if_arp.h> #ifdef CONFIG_INET #include <net/inet_common.h> @@ -2005,6 +2006,9 @@ static int tpacket_fill_skb(struct packet_sock *po, struct sk_buff *skb, if (unlikely(err)) return err; + if (dev->type == ARPHRD_ETHER) + skb->protocol = eth_type_trans(skb, dev); + data += dev->hard_header_len; to_write -= dev->hard_header_len; } @@ -2324,6 +2328,13 @@ static int packet_snd(struct socket *sock, sock_tx_timestamp(sk, &skb_shinfo(skb)->tx_flags); + if (dev->type == ARPHRD_ETHER) { + skb->protocol = eth_type_trans(skb, dev); + } else { + skb->protocol = proto; + skb->dev = dev; + } + if (!gso_type && (len > dev->mtu + reserve + extra_len)) { /* Earlier code assumed this would be a VLAN pkt, * double-check this now that we have the actual @@ -2338,8 +2349,6 @@ static int packet_snd(struct socket *sock, } } - skb->protocol = proto; - skb->dev = dev; skb->priority = sk->sk_priority; skb->mark = sk->sk_mark; -- 1.8.1.5 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 2/3] af_packet: fix for sending VLAN frames via packet_mmap 2013-08-02 9:37 ` [PATCH v2 " Phil Sutter @ 2013-08-02 9:37 ` Phil Sutter 2013-08-02 9:37 ` [PATCH v2 3/3] af_packet: simplify VLAN frame check in packet_snd Phil Sutter 2013-08-02 21:58 ` [PATCH v2 2/3] af_packet: fix for sending VLAN frames via packet_mmap David Miller 2013-08-02 21:58 ` [PATCH v2 1/3] af_packet: when sending ethernet frames, parse header for skb->protocol David Miller 1 sibling, 2 replies; 14+ messages in thread From: Phil Sutter @ 2013-08-02 9:37 UTC (permalink / raw) To: David S. Miller; +Cc: netdev Since tpacket_fill_skb() parses the protocol field in ethernet frames' headers, it's easy to see if any passed frame is a VLAN one and account for the extended size. But as the real protocol does not turn up before tpacket_fill_skb() runs which in turn also checks the frame length, move the max frame length calculation into the function. Signed-off-by: Phil Sutter <phil@nwl.cc> --- net/packet/af_packet.c | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c index e549e17..ce7b4f0 100644 --- a/net/packet/af_packet.c +++ b/net/packet/af_packet.c @@ -1924,7 +1924,7 @@ static int tpacket_fill_skb(struct packet_sock *po, struct sk_buff *skb, __be16 proto, unsigned char *addr, int hlen) { union tpacket_uhdr ph; - int to_write, offset, len, tp_len, nr_frags, len_max; + int to_write, offset, len, tp_len, nr_frags, len_max, max_frame_len; struct socket *sock = po->sk.sk_socket; struct page *page; void *data; @@ -1947,10 +1947,6 @@ static int tpacket_fill_skb(struct packet_sock *po, struct sk_buff *skb, tp_len = ph.h1->tp_len; break; } - if (unlikely(tp_len > size_max)) { - pr_err("packet size is too long (%d > %d)\n", tp_len, size_max); - return -EMSGSIZE; - } skb_reserve(skb, hlen); skb_reset_network_header(skb); @@ -2013,6 +2009,18 @@ static int tpacket_fill_skb(struct packet_sock *po, struct sk_buff *skb, to_write -= dev->hard_header_len; } + max_frame_len = dev->mtu + dev->hard_header_len; + if (skb->protocol == htons(ETH_P_8021Q)) + max_frame_len += VLAN_HLEN; + + if (size_max > max_frame_len) + size_max = max_frame_len; + + if (unlikely(tp_len > size_max)) { + pr_err("packet size is too long (%d > %d)\n", tp_len, size_max); + return -EMSGSIZE; + } + offset = offset_in_page(data); len_max = PAGE_SIZE - offset; len = ((to_write > len_max) ? len_max : to_write); @@ -2051,7 +2059,7 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg) struct net_device *dev; __be16 proto; bool need_rls_dev = false; - int err, reserve = 0; + int err; void *ph; struct sockaddr_ll *saddr = (struct sockaddr_ll *)msg->msg_name; int tp_len, size_max; @@ -2084,8 +2092,6 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg) if (unlikely(dev == NULL)) goto out; - reserve = dev->hard_header_len; - err = -ENETDOWN; if (unlikely(!(dev->flags & IFF_UP))) goto out_put; @@ -2093,9 +2099,6 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg) size_max = po->tx_ring.frame_size - (po->tp_hdrlen - sizeof(struct sockaddr_ll)); - if (size_max > dev->mtu + reserve) - size_max = dev->mtu + reserve; - do { ph = packet_current_frame(po, &po->tx_ring, TP_STATUS_SEND_REQUEST); -- 1.8.1.5 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 3/3] af_packet: simplify VLAN frame check in packet_snd 2013-08-02 9:37 ` [PATCH v2 2/3] af_packet: fix for sending VLAN frames via packet_mmap Phil Sutter @ 2013-08-02 9:37 ` Phil Sutter 2013-08-02 21:59 ` David Miller 2013-08-02 21:58 ` [PATCH v2 2/3] af_packet: fix for sending VLAN frames via packet_mmap David Miller 1 sibling, 1 reply; 14+ messages in thread From: Phil Sutter @ 2013-08-02 9:37 UTC (permalink / raw) To: David S. Miller; +Cc: netdev For ethernet frames, eth_type_trans() already parses the header, so one can skip this when checking the frame size. Signed-off-by: Phil Sutter <phil@nwl.cc> --- net/packet/af_packet.c | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c index ce7b4f0..bbe1ece 100644 --- a/net/packet/af_packet.c +++ b/net/packet/af_packet.c @@ -2333,23 +2333,16 @@ static int packet_snd(struct socket *sock, if (dev->type == ARPHRD_ETHER) { skb->protocol = eth_type_trans(skb, dev); + if (skb->protocol == htons(ETH_P_8021Q)) + reserve += VLAN_HLEN; } else { skb->protocol = proto; skb->dev = dev; } if (!gso_type && (len > dev->mtu + reserve + extra_len)) { - /* Earlier code assumed this would be a VLAN pkt, - * double-check this now that we have the actual - * packet in hand. - */ - struct ethhdr *ehdr; - skb_reset_mac_header(skb); - ehdr = eth_hdr(skb); - if (ehdr->h_proto != htons(ETH_P_8021Q)) { - err = -EMSGSIZE; - goto out_free; - } + err = -EMSGSIZE; + goto out_free; } skb->priority = sk->sk_priority; -- 1.8.1.5 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2 3/3] af_packet: simplify VLAN frame check in packet_snd 2013-08-02 9:37 ` [PATCH v2 3/3] af_packet: simplify VLAN frame check in packet_snd Phil Sutter @ 2013-08-02 21:59 ` David Miller 0 siblings, 0 replies; 14+ messages in thread From: David Miller @ 2013-08-02 21:59 UTC (permalink / raw) To: phil; +Cc: netdev From: Phil Sutter <phil@nwl.cc> Date: Fri, 2 Aug 2013 11:37:41 +0200 > For ethernet frames, eth_type_trans() already parses the header, so one > can skip this when checking the frame size. > > Signed-off-by: Phil Sutter <phil@nwl.cc> Applied. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/3] af_packet: fix for sending VLAN frames via packet_mmap 2013-08-02 9:37 ` [PATCH v2 2/3] af_packet: fix for sending VLAN frames via packet_mmap Phil Sutter 2013-08-02 9:37 ` [PATCH v2 3/3] af_packet: simplify VLAN frame check in packet_snd Phil Sutter @ 2013-08-02 21:58 ` David Miller 1 sibling, 0 replies; 14+ messages in thread From: David Miller @ 2013-08-02 21:58 UTC (permalink / raw) To: phil; +Cc: netdev From: Phil Sutter <phil@nwl.cc> Date: Fri, 2 Aug 2013 11:37:40 +0200 > Since tpacket_fill_skb() parses the protocol field in ethernet frames' > headers, it's easy to see if any passed frame is a VLAN one and account > for the extended size. > > But as the real protocol does not turn up before tpacket_fill_skb() > runs which in turn also checks the frame length, move the max frame > length calculation into the function. > > Signed-off-by: Phil Sutter <phil@nwl.cc> Applied. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/3] af_packet: when sending ethernet frames, parse header for skb->protocol 2013-08-02 9:37 ` [PATCH v2 " Phil Sutter 2013-08-02 9:37 ` [PATCH v2 2/3] af_packet: fix for sending VLAN frames via packet_mmap Phil Sutter @ 2013-08-02 21:58 ` David Miller 1 sibling, 0 replies; 14+ messages in thread From: David Miller @ 2013-08-02 21:58 UTC (permalink / raw) To: phil; +Cc: netdev From: Phil Sutter <phil@nwl.cc> Date: Fri, 2 Aug 2013 11:37:39 +0200 > This may be necessary when the SKB is passed to other layers on the go, > which check the protocol field on their own. An example is a VLAN packet > sent out using AF_PACKET on a bridge interface. The bridging code checks > the SKB size, accounting for any VLAN header only if the protocol field > is set accordingly. > > Note that eth_type_trans() sets skb->dev to the passed argument, so this > can be skipped in packet_snd() for ethernet frames, as well. > > Signed-off-by: Phil Sutter <phil@nwl.cc> Applied. ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2013-08-02 21:59 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-07-29 16:53 packet_mmap, 802.1Q and bridges Phil Sutter 2013-07-31 17:27 ` Parse ethernet headers in af_packet.c Phil Sutter 2013-07-31 17:27 ` [PATCH 1/3] af_packet: when sending ethernet frames, parse header for skb->protocol Phil Sutter 2013-07-31 17:27 ` [PATCH 2/3] af_packet: fix for sending VLAN frames via packet_mmap Phil Sutter 2013-07-31 17:27 ` [PATCH 3/3] af_packet: simplify VLAN frame check in packet_snd Phil Sutter 2013-08-01 0:30 ` [PATCH 1/3] af_packet: when sending ethernet frames, parse header for skb->protocol David Miller 2013-08-01 10:35 ` Phil Sutter 2013-08-02 1:12 ` David Miller 2013-08-02 9:37 ` [PATCH v2 " Phil Sutter 2013-08-02 9:37 ` [PATCH v2 2/3] af_packet: fix for sending VLAN frames via packet_mmap Phil Sutter 2013-08-02 9:37 ` [PATCH v2 3/3] af_packet: simplify VLAN frame check in packet_snd Phil Sutter 2013-08-02 21:59 ` David Miller 2013-08-02 21:58 ` [PATCH v2 2/3] af_packet: fix for sending VLAN frames via packet_mmap David Miller 2013-08-02 21:58 ` [PATCH v2 1/3] af_packet: when sending ethernet frames, parse header for skb->protocol David Miller
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).