netdev - Re: [PATCH net-next 1/2] macvtap: Add support of packet capture on macvtap device.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <52A9313E.6040602@redhat.com>
Date:	Thu, 12 Dec 2013 11:45:02 +0800
From:	Jason Wang <jasowang@...hat.com>
To:	Vlad Yasevich <vyasevic@...hat.com>, netdev@...r.kernel.org
CC:	mst@...hat.com, kaber@...sh.net
Subject: Re: [PATCH net-next 1/2] macvtap: Add support of packet capture on
 macvtap device.

On 12/12/2013 02:27 AM, Vlad Yasevich wrote:
> Macvtap device currently doesn not allow a user to capture
> traffic on due to the fact that it steals the packets
> from the network stack before the skb->dev is set correctly
> on the receive side, and that use uses macvlan transmit
> path directly on the send side.  As a result, we never
> get a change to give traffic to the taps while the correct
> device is set in the skb.
>
> This patch makes macvtap device behave almost exaclty like
> macvlan.  On the send side, we switch to using dev_queue_xmit().
> On the receive side, to deliver packets to macvtap, we now
> use netif_rx and dev_forward_skb just like macvlan.  The only
> differnce now is that macvtap has its own rx_handler which is
> attached to the macvtap netdev.  It is here that we now steal
> the packet and provide it to the socket.
>
> As a result, we can now capture traffic on the macvtap device:
>    tcpdump -i macvtap0
>
> It also gives us the abilit to add tc actions to the macvtap
> device and actually utilize different bandwidth management
> queues on output.
>
> Signed-off-by: Vlad Yasevich <vyasevic@...hat.com>
> ---
>  drivers/net/macvtap.c | 59 ++++++++++++++++++++++++++++-----------------------
>  1 file changed, 32 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
> index 4c6f84c..f9847da 100644
> --- a/drivers/net/macvtap.c
> +++ b/drivers/net/macvtap.c
> @@ -70,6 +70,11 @@ static const struct proto_ops macvtap_socket_ops;
>  #define RX_OFFLOADS (NETIF_F_GRO | NETIF_F_LRO)
>  #define TAP_FEATURES (NETIF_F_GSO | NETIF_F_SG)
>  
[...]
>  
>  static void macvtap_dellink(struct net_device *dev,
>  			    struct list_head *head)
>  {
> +	netdev_rx_handler_unregister(dev);
>  	macvtap_del_queues(dev);
>  	macvlan_dellink(dev, head);
>  }
> @@ -725,9 +731,8 @@ static ssize_t macvtap_get_user(struct macvtap_queue *q, struct msghdr *m,
>  		skb_shinfo(skb)->tx_flags |= SKBTX_SHARED_FRAG;
>  	}
>  	if (vlan) {
> -		local_bh_disable();
> -		macvlan_start_xmit(skb, vlan->dev);
> -		local_bh_enable();
> +		skb->dev = vlan->dev;
> +		dev_queue_xmit(skb);
>  	} else {
>  		kfree_skb(skb);
>  	}

An issue here: macvlan is a NETIF_F_LLTX device with only one transmit
queue, we may get contention on qdisc lock of macvtap when transmitting
through multiple tx queues.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html