Re: [PATCH net-next 1/2] macvlan: slight optimization for passthru mode

[Vlad Yasevich <vyasevich@gmail.com>]

On 05/27/2014 06:53 AM, Ding Tianhong wrote:
> When the maclvan is in passthru mode, there should be one(and only one)
> macvlan dev, so no need to check the vlan point, remove the judgement and
> the redundant variables.
> 
> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
> ---
>  drivers/net/macvlan.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
> index 55faff3..c9ada17 100644
> --- a/drivers/net/macvlan.c
> +++ b/drivers/net/macvlan.c
> @@ -274,7 +274,6 @@ static rx_handler_result_t macvlan_handle_frame(struct sk_buff **pskb)
>  	const struct ethhdr *eth = eth_hdr(skb);
>  	const struct macvlan_dev *vlan;
>  	const struct macvlan_dev *src;
> -	struct net_device *dev;
>  	unsigned int len = 0;
>  	int ret = NET_RX_DROP;
>  
> @@ -300,16 +299,16 @@ static rx_handler_result_t macvlan_handle_frame(struct sk_buff **pskb)
>  		return RX_HANDLER_PASS;
>  	}
>  
> -	if (port->passthru)
> +	if (port->passthru) {
>  		vlan = list_first_or_null_rcu(&port->vlans,
>  					      struct macvlan_dev, list);
> -	else
> +	} else {
>  		vlan = macvlan_hash_lookup(port, eth->h_dest);
> -	if (vlan == NULL)
> -		return RX_HANDLER_PASS;
> +		if (vlan == NULL)
> +			return RX_HANDLER_PASS;
> +	}
>  

I don't think you can do that.  del_link() happens before ndo_uninit(),
and we unregister the rx_handler in ndo_uninit().  As such it is
possible to enter the rx_handler while the ports->vlans is empty.

-vlad

> -	dev = vlan->dev;
> -	if (unlikely(!(dev->flags & IFF_UP))) {
> +	if (unlikely(!(vlan->dev->flags & IFF_UP))) {
>  		kfree_skb(skb);
>  		return RX_HANDLER_CONSUMED;
>  	}
> @@ -318,7 +317,7 @@ static rx_handler_result_t macvlan_handle_frame(struct sk_buff **pskb)
>  	if (!skb)
>  		goto out;
>  
> -	skb->dev = dev;
> +	skb->dev = vlan->dev;
>  	skb->pkt_type = PACKET_HOST;
>  
>  	ret = netif_rx(skb);
>