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: [email protected]
---
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
On Mon, Sep 30, 2019 at 1:24 AM Sriram Krishnan <[email protected]> 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: [email protected]
> ---
> 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.
On Mon, 30 Sep 2019 11:16:14 -0400
Willem de Bruijn <[email protected]> wrote:
> On Mon, Sep 30, 2019 at 1:24 AM Sriram Krishnan <[email protected]> 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: [email protected]
> > ---
> > 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.
On Tue, Oct 1, 2019 at 11:44 AM Stephen Hemminger
<[email protected]> wrote:
>
> On Mon, 30 Sep 2019 11:16:14 -0400
> Willem de Bruijn <[email protected]> wrote:
>
> > On Mon, Sep 30, 2019 at 1:24 AM Sriram Krishnan <[email protected]> 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.