From: ebiederm@xmission•com (Eric W. Biederman)
To: Basil Gor <basil.gor@gmail•com>
Cc: netdev@vger•kernel.org, "David S. Miller" <davem@davemloft•net>,
Basil Gor <basilgor@gmail•com>
Subject: Re: [PATCH] macvlan/macvtap: Fix vlan tagging on user read
Date: Wed, 18 Apr 2012 11:54:52 -0700 [thread overview]
Message-ID: <m1hawglvk3.fsf@fess.ebiederm.org> (raw)
In-Reply-To: <1334774098-22886-1-git-send-email-basilgor@gmail.com> (Basil Gor's message of "Wed, 18 Apr 2012 22:34:58 +0400")
Basil Gor <basil.gor@gmail•com> writes:
> Vlan tag is restored during buffer transmit to a network device (bridge
> port) in bridging code in case of tun/tap driver. In case of macvtap it
> has to be done explicitly. Otherwise vlan_tci is ignored and user always
> gets untagged packets.
>
> Scenario tested:
> kvm guests (that use vlans) migration from bridged network to macvtap
> revealed that packets delivered to guests are always untagged. Dumping
> and comparing sk_buff in case of tap and macvtap driver showed that
> macvtap does not restore vlan_tci.
>
> With current patch applied I was able to get working network, kvm guests
> get correctly tagged packets and can reach each other when macvtap in
> bridge mode (both with no vlans and through vlan interfaces).
My first impression is that this is the wrong place to add a vlan
header back.
You need to keep the vlan information in vlan_tci until just
before the packet is delivered to userspace. Which would suggest
the best place for these games is macvtap_put_user.
Elsewhere vlan headers should not be explicitly stored in the packet.
At least that was the rule last I looked.
Eric
> Signed-off-by: Basil Gor <basilgor@gmail•com>
> ---
> drivers/net/macvtap.c | 9 +++++++++
> 1 files changed, 9 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
> index 0427c65..a6802b9 100644
> --- a/drivers/net/macvtap.c
> +++ b/drivers/net/macvtap.c
> @@ -1,6 +1,7 @@
> #include <linux/etherdevice.h>
> #include <linux/if_macvlan.h>
> #include <linux/interrupt.h>
> +#include <linux/if_vlan.h>
> #include <linux/nsproxy.h>
> #include <linux/compat.h>
> #include <linux/if_tun.h>
> @@ -254,6 +255,14 @@ static int macvtap_forward(struct net_device *dev, struct sk_buff *skb)
> if (skb_queue_len(&q->sk.sk_receive_queue) >= dev->tx_queue_len)
> goto drop;
>
> + if (vlan_tx_tag_present(skb)) {
> + skb = __vlan_put_tag(skb, vlan_tx_tag_get(skb));
> + if (unlikely(!skb))
> + return NET_RX_DROP;
> +
> + skb->vlan_tci = 0;
> + }
> +
> skb_queue_tail(&q->sk.sk_receive_queue, skb);
> wake_up_interruptible_poll(sk_sleep(&q->sk), POLLIN | POLLRDNORM | POLLRDBAND);
> return NET_RX_SUCCESS;
next prev parent reply other threads:[~2012-04-18 18:51 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-04-18 18:34 [PATCH] macvlan/macvtap: Fix vlan tagging on user read Basil Gor
2012-04-18 18:54 ` Eric W. Biederman [this message]
2012-04-18 19:33 ` Basil Gor
2012-04-20 23:11 ` Basil Gor
2012-04-21 1:49 ` Eric W. Biederman
2012-04-25 17:01 ` [PATCH v3 1/2] vhost-net: fix handle_rx buffer size Basil Gor
2012-04-26 5:30 ` Eric W. Biederman
2012-05-03 12:39 ` Michael S. Tsirkin
2012-05-03 13:16 ` Michael S. Tsirkin
2012-05-03 14:43 ` Basil Gor
2012-04-25 17:01 ` [PATCH v3 2/2] macvtap: restore vlan header on user read Basil Gor
2012-04-26 5:31 ` Eric W. Biederman
2012-05-03 13:07 ` Michael S. Tsirkin
2012-05-03 13:37 ` Eric W. Biederman
2012-05-03 14:31 ` Michael S. Tsirkin
2012-05-03 15:22 ` Basil Gor
2012-05-03 23:11 ` Basil Gor
2012-05-03 23:30 ` Michael S. Tsirkin
2012-05-03 23:47 ` Basil Gor
2012-05-04 1:06 ` David Miller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=m1hawglvk3.fsf@fess.ebiederm.org \
--to=ebiederm@xmission$(echo .)com \
--cc=basil.gor@gmail$(echo .)com \
--cc=basilgor@gmail$(echo .)com \
--cc=davem@davemloft$(echo .)net \
--cc=netdev@vger$(echo .)kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox