* [PATCH] AF_PACKET doesnt strip VLAN information
@ 2019-09-28 4:58 Sriram Krishnan
2019-09-30 15:16 ` Willem de Bruijn
0 siblings, 1 reply; 4+ messages in thread
From: Sriram Krishnan @ 2019-09-28 4:58 UTC (permalink / raw)
To: Andrew Morton; +Cc: xe-linux-external, David S. Miller, netdev, linux-kernel
When an application sends with AF_PACKET and places a vlan header on
the raw packet; then the AF_PACKET needs to move the tag into the skb
so that it gets processed normally through the rest of the transmit
path.
This is particularly a problem on Hyper-V where the host only allows
vlan in the offload info.
Cc: xe-linux-external@cisco.com
---
net/packet/af_packet.c | 26 ++++++++++++++++++++++++--
1 file changed, 24 insertions(+), 2 deletions(-)
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index e2742b0..cfe0904 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -1849,15 +1849,35 @@ static int packet_rcv_spkt(struct sk_buff *skb, struct net_device *dev,
return 0;
}
-static void packet_parse_headers(struct sk_buff *skb, struct socket *sock)
+static int packet_parse_headers(struct sk_buff *skb, struct socket *sock)
{
if ((!skb->protocol || skb->protocol == htons(ETH_P_ALL)) &&
sock->type == SOCK_RAW) {
+ __be16 ethertype;
+
skb_reset_mac_header(skb);
+
+ ethertype = eth_hdr(skb)->h_proto;
+ /*
+ * If Vlan tag is present in the packet
+ * move it to skb
+ */
+ if (eth_type_vlan(ethertype)) {
+ int err;
+ __be16 vlan_tci;
+
+ err = __skb_vlan_pop(skb, &vlan_tci);
+ if (unlikely(err))
+ return err;
+
+ __vlan_hwaccel_put_tag(skb, ethertype, vlan_tci);
+ }
+
skb->protocol = dev_parse_header_protocol(skb);
}
skb_probe_transport_header(skb);
+ return 0;
}
/*
@@ -1979,7 +1999,9 @@ static int packet_sendmsg_spkt(struct socket *sock, struct msghdr *msg,
if (unlikely(extra_len == 4))
skb->no_fcs = 1;
- packet_parse_headers(skb, sock);
+ err = packet_parse_headers(skb, sock);
+ if (err)
+ goto out_unlock;
dev_queue_xmit(skb);
rcu_read_unlock();
--
2.7.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] AF_PACKET doesnt strip VLAN information
2019-09-28 4:58 [PATCH] AF_PACKET doesnt strip VLAN information Sriram Krishnan
@ 2019-09-30 15:16 ` Willem de Bruijn
2019-10-01 15:44 ` Stephen Hemminger
0 siblings, 1 reply; 4+ messages in thread
From: Willem de Bruijn @ 2019-09-30 15:16 UTC (permalink / raw)
To: Sriram Krishnan
Cc: Andrew Morton, xe-linux-external, David S. Miller,
Network Development, linux-kernel
On Mon, Sep 30, 2019 at 1:24 AM Sriram Krishnan <srirakr2@cisco.com> wrote:
>
> When an application sends with AF_PACKET and places a vlan header on
> the raw packet; then the AF_PACKET needs to move the tag into the skb
> so that it gets processed normally through the rest of the transmit
> path.
>
> This is particularly a problem on Hyper-V where the host only allows
> vlan in the offload info.
This sounds like behavior that needs to be addressed in the driver, instead?
> Cc: xe-linux-external@cisco.com
> ---
> net/packet/af_packet.c | 26 ++++++++++++++++++++++++--
> 1 file changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> index e2742b0..cfe0904 100644
> --- a/net/packet/af_packet.c
> +++ b/net/packet/af_packet.c
> @@ -1849,15 +1849,35 @@ static int packet_rcv_spkt(struct sk_buff *skb, struct net_device *dev,
> return 0;
> }
>
> -static void packet_parse_headers(struct sk_buff *skb, struct socket *sock)
> +static int packet_parse_headers(struct sk_buff *skb, struct socket *sock)
> {
> if ((!skb->protocol || skb->protocol == htons(ETH_P_ALL)) &&
> sock->type == SOCK_RAW) {
If inside this branch, may miss packets with skb->protocol set to one
of the VLAN Ethertypes.
> + __be16 ethertype;
> +
> skb_reset_mac_header(skb);
> +
> + ethertype = eth_hdr(skb)->h_proto;
> + /*
> + * If Vlan tag is present in the packet
> + * move it to skb
> + */
> + if (eth_type_vlan(ethertype)) {
> + int err;
> + __be16 vlan_tci;
> +
> + err = __skb_vlan_pop(skb, &vlan_tci);
> + if (unlikely(err))
> + return err;
> +
> + __vlan_hwaccel_put_tag(skb, ethertype, vlan_tci);
What happens with multiple tags (QinQ)?
> + }
> +
> skb->protocol = dev_parse_header_protocol(skb);
> }
>
> skb_probe_transport_header(skb);
> + return 0;
> }
>
> /*
> @@ -1979,7 +1999,9 @@ static int packet_sendmsg_spkt(struct socket *sock, struct msghdr *msg,
> if (unlikely(extra_len == 4))
> skb->no_fcs = 1;
>
> - packet_parse_headers(skb, sock);
> + err = packet_parse_headers(skb, sock);
> + if (err)
> + goto out_unlock;
This only tests the new return value in one of three callers of
packet_sendmsg_spkt.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] AF_PACKET doesnt strip VLAN information
2019-09-30 15:16 ` Willem de Bruijn
@ 2019-10-01 15:44 ` Stephen Hemminger
2019-10-01 15:52 ` Willem de Bruijn
0 siblings, 1 reply; 4+ messages in thread
From: Stephen Hemminger @ 2019-10-01 15:44 UTC (permalink / raw)
To: Willem de Bruijn
Cc: Sriram Krishnan, Andrew Morton, xe-linux-external,
David S. Miller, Network Development, linux-kernel
On Mon, 30 Sep 2019 11:16:14 -0400
Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote:
> On Mon, Sep 30, 2019 at 1:24 AM Sriram Krishnan <srirakr2@cisco.com> wrote:
> >
> > When an application sends with AF_PACKET and places a vlan header on
> > the raw packet; then the AF_PACKET needs to move the tag into the skb
> > so that it gets processed normally through the rest of the transmit
> > path.
> >
> > This is particularly a problem on Hyper-V where the host only allows
> > vlan in the offload info.
>
> This sounds like behavior that needs to be addressed in the driver, instead?
This was what we did first, but the problem was more general.
For example, many filtering functions assume that vlan tag is in
skb meta data, not the packet data itself. Therefore AF_PACKET would
get around any filter rules.
>
> > Cc: xe-linux-external@cisco.com
> > ---
> > net/packet/af_packet.c | 26 ++++++++++++++++++++++++--
> > 1 file changed, 24 insertions(+), 2 deletions(-)
> >
> > diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> > index e2742b0..cfe0904 100644
> > --- a/net/packet/af_packet.c
> > +++ b/net/packet/af_packet.c
> > @@ -1849,15 +1849,35 @@ static int packet_rcv_spkt(struct sk_buff *skb, struct net_device *dev,
> > return 0;
> > }
> >
> > -static void packet_parse_headers(struct sk_buff *skb, struct socket *sock)
> > +static int packet_parse_headers(struct sk_buff *skb, struct socket *sock)
> > {
> > if ((!skb->protocol || skb->protocol == htons(ETH_P_ALL)) &&
> > sock->type == SOCK_RAW) {
>
> If inside this branch, may miss packets with skb->protocol set to one
> of the VLAN Ethertypes.
>
> > + __be16 ethertype;
> > +
> > skb_reset_mac_header(skb);
> > +
> > + ethertype = eth_hdr(skb)->h_proto;
> > + /*
> > + * If Vlan tag is present in the packet
> > + * move it to skb
> > + */
> > + if (eth_type_vlan(ethertype)) {
> > + int err;
> > + __be16 vlan_tci;
> > +
> > + err = __skb_vlan_pop(skb, &vlan_tci);
> > + if (unlikely(err))
> > + return err;
> > +
> > + __vlan_hwaccel_put_tag(skb, ethertype, vlan_tci);
>
> What happens with multiple tags (QinQ)?
Same as multiple tags in a normal sent packet. The second tag is in
the packet itself.
>
> > + }
> > +
> > skb->protocol = dev_parse_header_protocol(skb);
> > }
> >
> > skb_probe_transport_header(skb);
> > + return 0;
> > }
> >
> > /*
> > @@ -1979,7 +1999,9 @@ static int packet_sendmsg_spkt(struct socket *sock, struct msghdr *msg,
> > if (unlikely(extra_len == 4))
> > skb->no_fcs = 1;
> >
> > - packet_parse_headers(skb, sock);
> > + err = packet_parse_headers(skb, sock);
> > + if (err)
> > + goto out_unlock;
>
> This only tests the new return value in one of three callers of
> packet_sendmsg_spkt.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] AF_PACKET doesnt strip VLAN information
2019-10-01 15:44 ` Stephen Hemminger
@ 2019-10-01 15:52 ` Willem de Bruijn
0 siblings, 0 replies; 4+ messages in thread
From: Willem de Bruijn @ 2019-10-01 15:52 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Willem de Bruijn, Sriram Krishnan, Andrew Morton,
xe-linux-external, David S. Miller, Network Development,
linux-kernel
On Tue, Oct 1, 2019 at 11:44 AM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Mon, 30 Sep 2019 11:16:14 -0400
> Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote:
>
> > On Mon, Sep 30, 2019 at 1:24 AM Sriram Krishnan <srirakr2@cisco.com> wrote:
> > >
> > > When an application sends with AF_PACKET and places a vlan header on
> > > the raw packet; then the AF_PACKET needs to move the tag into the skb
> > > so that it gets processed normally through the rest of the transmit
> > > path.
> > >
> > > This is particularly a problem on Hyper-V where the host only allows
> > > vlan in the offload info.
> >
> > This sounds like behavior that needs to be addressed in the driver, instead?
>
> This was what we did first, but the problem was more general.
> For example, many filtering functions assume that vlan tag is in
> skb meta data, not the packet data itself.
Out of curiosity, can you share an example?
> Therefore AF_PACKET would
> get around any filter rules.
Packet sockets are not the only way to inject packets into the kernel.
This probably also affects tap.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-10-01 15:52 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-28 4:58 [PATCH] AF_PACKET doesnt strip VLAN information Sriram Krishnan
2019-09-30 15:16 ` Willem de Bruijn
2019-10-01 15:44 ` Stephen Hemminger
2019-10-01 15:52 ` Willem de Bruijn
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).