Netdev List
 help / color / mirror / Atom feed
* Re: [patch net-next-2.6 3/8] net: get rid of multiple bond-related netdevice->priv_flags
From: Nicolas de Pesloüan @ 2011-03-05 14:14 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: netdev, davem, shemminger, kaber, fubar, eric.dumazet, andy
In-Reply-To: <1299320969-7951-4-git-send-email-jpirko@redhat.com>

Le 05/03/2011 11:29, Jiri Pirko a écrit :
> Now when bond-related code is moved from net/core/dev.c into bonding
> code, multiple priv_flags are not needed anymore. So toss them out.

Agreed. See at the end for a single comment.

>
> Signed-off-by: Jiri Pirko<jpirko@redhat.com>
> ---
>   drivers/net/bonding/bond_main.c  |   33 ++++++++++++++-------------------
>   drivers/net/bonding/bond_sysfs.c |    8 --------
>   drivers/net/bonding/bonding.h    |   24 +-----------------------
>   include/linux/if.h               |   22 +++++++++-------------
>   4 files changed, 24 insertions(+), 63 deletions(-)
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 1c19368..7923184 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -1471,20 +1471,20 @@ static void bond_setup_by_slave(struct net_device *bond_dev,
>    * ARP on active-backup slaves with arp_validate enabled.
>    */
>   static bool bond_should_deliver_exact_match(struct sk_buff *skb,
> -					    struct net_device *slave_dev,
> -					    struct net_device *bond_dev)
> +					    struct slave *slave,
> +					    struct bonding *bond)
>   {
> -	if (slave_dev->priv_flags&  IFF_SLAVE_INACTIVE) {
> -		if (slave_dev->priv_flags&  IFF_SLAVE_NEEDARP&&
> +	if (slave->dev->priv_flags&  IFF_SLAVE_INACTIVE) {
> +		if (slave_do_arp_validate(bond, slave)&&
>   		skb->protocol == __cpu_to_be16(ETH_P_ARP))
>   			return false;
>
> -		if (bond_dev->priv_flags&  IFF_MASTER_ALB&&
> +		if (bond->params.mode == BOND_MODE_ALB&&
>   		skb->pkt_type != PACKET_BROADCAST&&
>   		skb->pkt_type != PACKET_MULTICAST)
>   				return false;
>
> -		if (bond_dev->priv_flags&  IFF_MASTER_8023AD&&
> +		if (bond->params.mode == BOND_MODE_8023AD&&
>   		skb->protocol == __cpu_to_be16(ETH_P_SLOW))
>   			return false;
>
> @@ -1497,6 +1497,7 @@ static struct sk_buff *bond_handle_frame(struct sk_buff *skb)
>   {
>   	struct slave *slave;
>   	struct net_device *bond_dev;
> +	struct bonding *bond;
>
>   	skb = skb_share_check(skb, GFP_ATOMIC);
>   	if (unlikely(!skb))
> @@ -1507,17 +1508,19 @@ static struct sk_buff *bond_handle_frame(struct sk_buff *skb)
>   	if (unlikely(!bond_dev))
>   		return skb;
>
> -	if (bond_dev->priv_flags&  IFF_MASTER_ARPMON)
> +	bond = netdev_priv(bond_dev);
> +
> +	if (bond->params.arp_interval)
>   		slave->dev->last_rx = jiffies;
>
> -	if (bond_should_deliver_exact_match(skb, slave->dev, bond_dev)) {
> +	if (bond_should_deliver_exact_match(skb, slave, bond)) {
>   		skb->deliver_no_wcard = 1;
>   		return skb;
>   	}
>
>   	skb->dev = bond_dev;
>
> -	if (bond_dev->priv_flags&  IFF_MASTER_ALB&&
> +	if (bond->params.mode == BOND_MODE_ALB&&
>   	bond_dev->priv_flags&  IFF_BRIDGE_PORT&&
>   	skb->pkt_type == PACKET_HOST) {
>   		u16 *dest = (u16 *) eth_hdr(skb)->h_dest;
> @@ -2128,9 +2131,7 @@ int bond_release(struct net_device *bond_dev, struct net_device *slave_dev)
>
>   	dev_set_mtu(slave_dev, slave->original_mtu);
>
> -	slave_dev->priv_flags&= ~(IFF_MASTER_8023AD | IFF_MASTER_ALB |
> -				   IFF_SLAVE_INACTIVE | IFF_BONDING |
> -				   IFF_SLAVE_NEEDARP);
> +	slave_dev->priv_flags&= ~(IFF_SLAVE_INACTIVE | IFF_BONDING);
>
>   	kfree(slave);
>
> @@ -2241,8 +2242,7 @@ static int bond_release_all(struct net_device *bond_dev)
>   			dev_set_mac_address(slave_dev,&addr);
>   		}
>
> -		slave_dev->priv_flags&= ~(IFF_MASTER_8023AD | IFF_MASTER_ALB |
> -					   IFF_SLAVE_INACTIVE);
> +		slave_dev->priv_flags&= ~IFF_SLAVE_INACTIVE;
>
>   		kfree(slave);
>
> @@ -4718,11 +4718,9 @@ void bond_set_mode_ops(struct bonding *bond, int mode)
>   	case BOND_MODE_BROADCAST:
>   		break;
>   	case BOND_MODE_8023AD:
> -		bond_set_master_3ad_flags(bond);
>   		bond_set_xmit_hash_policy(bond);
>   		break;
>   	case BOND_MODE_ALB:
> -		bond_set_master_alb_flags(bond);
>   		/* FALLTHRU */
>   	case BOND_MODE_TLB:
>   		break;
> @@ -4813,9 +4811,6 @@ static void bond_setup(struct net_device *bond_dev)
>   	bond_dev->priv_flags |= IFF_BONDING;
>   	bond_dev->priv_flags&= ~IFF_XMIT_DST_RELEASE;
>
> -	if (bond->params.arp_interval)
> -		bond_dev->priv_flags |= IFF_MASTER_ARPMON;
> -
>   	/* At first, we block adding VLANs. That's the only way to
>   	 * prevent problems that occur when adding VLANs over an
>   	 * empty bond. The block will be removed once non-challenged
> diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
> index 72bb0f6..05e0ae5 100644
> --- a/drivers/net/bonding/bond_sysfs.c
> +++ b/drivers/net/bonding/bond_sysfs.c
> @@ -322,11 +322,6 @@ static ssize_t bonding_store_mode(struct device *d,
>   		ret = -EINVAL;
>   		goto out;
>   	}
> -	if (bond->params.mode == BOND_MODE_8023AD)
> -		bond_unset_master_3ad_flags(bond);
> -
> -	if (bond->params.mode == BOND_MODE_ALB)
> -		bond_unset_master_alb_flags(bond);
>
>   	bond->params.mode = new_value;
>   	bond_set_mode_ops(bond, bond->params.mode);
> @@ -527,8 +522,6 @@ static ssize_t bonding_store_arp_interval(struct device *d,
>   	pr_info("%s: Setting ARP monitoring interval to %d.\n",
>   		bond->dev->name, new_value);
>   	bond->params.arp_interval = new_value;
> -	if (bond->params.arp_interval)
> -		bond->dev->priv_flags |= IFF_MASTER_ARPMON;
>   	if (bond->params.miimon) {
>   		pr_info("%s: ARP monitoring cannot be used with MII monitoring. %s Disabling MII monitoring.\n",
>   			bond->dev->name, bond->dev->name);
> @@ -1004,7 +997,6 @@ static ssize_t bonding_store_miimon(struct device *d,
>   			pr_info("%s: MII monitoring cannot be used with ARP monitoring. Disabling ARP monitoring...\n",
>   				bond->dev->name);
>   			bond->params.arp_interval = 0;
> -			bond->dev->priv_flags&= ~IFF_MASTER_ARPMON;
>   			if (bond->params.arp_validate) {
>   				bond_unregister_arp(bond);
>   				bond->params.arp_validate =
> diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
> index 1aac5cd..ddee62f 100644
> --- a/drivers/net/bonding/bonding.h
> +++ b/drivers/net/bonding/bonding.h
> @@ -354,34 +354,12 @@ static inline void bond_set_slave_inactive_flags(struct slave *slave)
>   		slave->state = BOND_STATE_BACKUP;
>   	if (!bond->params.all_slaves_active)
>   		slave->dev->priv_flags |= IFF_SLAVE_INACTIVE;
> -	if (slave_do_arp_validate(bond, slave))
> -		slave->dev->priv_flags |= IFF_SLAVE_NEEDARP;
>   }
>
>   static inline void bond_set_slave_active_flags(struct slave *slave)
>   {
>   	slave->state = BOND_STATE_ACTIVE;
> -	slave->dev->priv_flags&= ~(IFF_SLAVE_INACTIVE | IFF_SLAVE_NEEDARP);
> -}
> -
> -static inline void bond_set_master_3ad_flags(struct bonding *bond)
> -{
> -	bond->dev->priv_flags |= IFF_MASTER_8023AD;
> -}
> -
> -static inline void bond_unset_master_3ad_flags(struct bonding *bond)
> -{
> -	bond->dev->priv_flags&= ~IFF_MASTER_8023AD;
> -}
> -
> -static inline void bond_set_master_alb_flags(struct bonding *bond)
> -{
> -	bond->dev->priv_flags |= IFF_MASTER_ALB;
> -}
> -
> -static inline void bond_unset_master_alb_flags(struct bonding *bond)
> -{
> -	bond->dev->priv_flags&= ~IFF_MASTER_ALB;
> +	slave->dev->priv_flags&= ~IFF_SLAVE_INACTIVE;
>   }
>
>   struct vlan_entry *bond_next_vlan(struct bonding *bond, struct vlan_entry *curr);
> diff --git a/include/linux/if.h b/include/linux/if.h
> index 3bc63e6..2fdd47a 100644
> --- a/include/linux/if.h
> +++ b/include/linux/if.h
> @@ -60,21 +60,17 @@
>   #define IFF_802_1Q_VLAN 0x1             /* 802.1Q VLAN device.          */
>   #define IFF_EBRIDGE	0x2		/* Ethernet bridging device.	*/
>   #define IFF_SLAVE_INACTIVE	0x4	/* bonding slave not the curr. active */
> -#define IFF_MASTER_8023AD	0x8	/* bonding master, 802.3ad. 	*/
> -#define IFF_MASTER_ALB	0x10		/* bonding master, balance-alb.	*/
> -#define IFF_BONDING	0x20		/* bonding master or slave	*/
> -#define IFF_SLAVE_NEEDARP 0x40		/* need ARPs for validation	*/
> -#define IFF_ISATAP	0x80		/* ISATAP interface (RFC4214)	*/
> -#define IFF_MASTER_ARPMON 0x100		/* bonding master, ARP mon in use */
> -#define IFF_WAN_HDLC	0x200		/* WAN HDLC device		*/
> -#define IFF_XMIT_DST_RELEASE 0x400	/* dev_hard_start_xmit() is allowed to
> +#define IFF_BONDING	0x8		/* bonding master or slave	*/
> +#define IFF_ISATAP	0x10		/* ISATAP interface (RFC4214)	*/
> +#define IFF_WAN_HDLC	0x20		/* WAN HDLC device		*/
> +#define IFF_XMIT_DST_RELEASE 0x40	/* dev_hard_start_xmit() is allowed to
>   					 * release skb->dst
>   					 */

Why did you changed those values? Aren't those exposed to userland? Just removing the unused one 
sounds good to me.

> -#define IFF_DONT_BRIDGE 0x800		/* disallow bridging this ether dev */
> -#define IFF_DISABLE_NETPOLL	0x1000	/* disable netpoll at run-time */
> -#define IFF_MACVLAN_PORT	0x2000	/* device used as macvlan port */
> -#define IFF_BRIDGE_PORT	0x4000		/* device used as bridge port */
> -#define IFF_OVS_DATAPATH	0x8000	/* device used as Open vSwitch
> +#define IFF_DONT_BRIDGE 0x80		/* disallow bridging this ether dev */
> +#define IFF_DISABLE_NETPOLL	0x100	/* disable netpoll at run-time */
> +#define IFF_MACVLAN_PORT	0x200	/* device used as macvlan port */
> +#define IFF_BRIDGE_PORT	0x400		/* device used as bridge port */
> +#define IFF_OVS_DATAPATH	0x800	/* device used as Open vSwitch
>   					 * datapath port */
>
>   #define IF_GET_IFACE	0x0001		/* for querying only */


^ permalink raw reply

* Re: [patch net-next-2.6 5/8] bonding: get rid of IFF_SLAVE_INACTIVE netdev->priv_flag
From: Nicolas de Pesloüan @ 2011-03-05 14:18 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: netdev, davem, shemminger, kaber, fubar, eric.dumazet, andy
In-Reply-To: <1299320969-7951-6-git-send-email-jpirko@redhat.com>

Le 05/03/2011 11:29, Jiri Pirko a écrit :
> Since bond-related code was moved from net/core/dev.c into bonding,
> IFF_SLAVE_INACTIVE is no longer needed. Replace is with flag "inactive"
> stored in slave structure
>
> Signed-off-by: Jiri Pirko<jpirko@redhat.com>

Reviewed-by: Nicolas de Pesloüan <nicolas.2p.debian@free.fr>

> ---
>   drivers/net/bonding/bond_main.c  |    6 ++----
>   drivers/net/bonding/bond_sysfs.c |    4 ++--
>   drivers/net/bonding/bonding.h    |   14 ++++++++++----
>   include/linux/if.h               |   19 +++++++++----------
>   4 files changed, 23 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 62020a7..dbe182c 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -1474,7 +1474,7 @@ static bool bond_should_deliver_exact_match(struct sk_buff *skb,
>   					    struct slave *slave,
>   					    struct bonding *bond)
>   {
> -	if (slave->dev->priv_flags&  IFF_SLAVE_INACTIVE) {
> +	if (bond_is_slave_inactive(slave)) {
>   		if (slave_do_arp_validate(bond, slave)&&
>   		skb->protocol == __cpu_to_be16(ETH_P_ARP))
>   			return false;
> @@ -2131,7 +2131,7 @@ int bond_release(struct net_device *bond_dev, struct net_device *slave_dev)
>
>   	dev_set_mtu(slave_dev, slave->original_mtu);
>
> -	slave_dev->priv_flags&= ~(IFF_SLAVE_INACTIVE | IFF_BONDING);
> +	slave_dev->priv_flags&= ~IFF_BONDING;
>
>   	kfree(slave);
>
> @@ -2242,8 +2242,6 @@ static int bond_release_all(struct net_device *bond_dev)
>   			dev_set_mac_address(slave_dev,&addr);
>   		}
>
> -		slave_dev->priv_flags&= ~IFF_SLAVE_INACTIVE;
> -
>   		kfree(slave);
>
>   		/* re-acquire the lock before getting the next slave */
> diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
> index 344d23f..5161183 100644
> --- a/drivers/net/bonding/bond_sysfs.c
> +++ b/drivers/net/bonding/bond_sysfs.c
> @@ -1581,9 +1581,9 @@ static ssize_t bonding_store_slaves_active(struct device *d,
>   	bond_for_each_slave(bond, slave, i) {
>   		if (!bond_is_active_slave(slave)) {
>   			if (new_value)
> -				slave->dev->priv_flags&= ~IFF_SLAVE_INACTIVE;
> +				slave->inactive = 0;
>   			else
> -				slave->dev->priv_flags |= IFF_SLAVE_INACTIVE;
> +				slave->inactive = 1;
>   		}
>   	}
>   out:
> diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
> index 8a3718b..8f78166 100644
> --- a/drivers/net/bonding/bonding.h
> +++ b/drivers/net/bonding/bonding.h
> @@ -190,8 +190,9 @@ struct slave {
>   	unsigned long last_arp_rx;
>   	s8     link;    /* one of BOND_LINK_XXXX */
>   	s8     new_link;
> -	u8     backup;	/* indicates backup slave. Value corresponds with
> -			   BOND_STATE_ACTIVE and BOND_STATE_BACKUP */
> +	u8     backup:1,   /* indicates backup slave. Value corresponds with
> +			      BOND_STATE_ACTIVE and BOND_STATE_BACKUP */
> +	       inactive:1; /* indicates inactive slave */
>   	u32    original_mtu;
>   	u32    link_failure_count;
>   	u8     perm_hwaddr[ETH_ALEN];
> @@ -374,13 +375,18 @@ static inline void bond_set_slave_inactive_flags(struct slave *slave)
>   	if (!bond_is_lb(bond))
>   		bond_set_backup_slave(slave);
>   	if (!bond->params.all_slaves_active)
> -		slave->dev->priv_flags |= IFF_SLAVE_INACTIVE;
> +		slave->inactive = 1;
>   }
>
>   static inline void bond_set_slave_active_flags(struct slave *slave)
>   {
>   	bond_set_active_slave(slave);
> -	slave->dev->priv_flags&= ~IFF_SLAVE_INACTIVE;
> +	slave->inactive = 0;
> +}
> +
> +static inline bool bond_is_slave_inactive(struct slave *slave)
> +{
> +	return slave->inactive;
>   }
>
>   struct vlan_entry *bond_next_vlan(struct bonding *bond, struct vlan_entry *curr);
> diff --git a/include/linux/if.h b/include/linux/if.h
> index 2fdd47a..0da7991 100644
> --- a/include/linux/if.h
> +++ b/include/linux/if.h
> @@ -59,18 +59,17 @@
>   /* Private (from user) interface flags (netdevice->priv_flags). */
>   #define IFF_802_1Q_VLAN 0x1             /* 802.1Q VLAN device.          */
>   #define IFF_EBRIDGE	0x2		/* Ethernet bridging device.	*/
> -#define IFF_SLAVE_INACTIVE	0x4	/* bonding slave not the curr. active */
> -#define IFF_BONDING	0x8		/* bonding master or slave	*/
> -#define IFF_ISATAP	0x10		/* ISATAP interface (RFC4214)	*/
> -#define IFF_WAN_HDLC	0x20		/* WAN HDLC device		*/
> -#define IFF_XMIT_DST_RELEASE 0x40	/* dev_hard_start_xmit() is allowed to
> +#define IFF_BONDING	0x4		/* bonding master or slave	*/
> +#define IFF_ISATAP	0x8		/* ISATAP interface (RFC4214)	*/
> +#define IFF_WAN_HDLC	0x10		/* WAN HDLC device		*/
> +#define IFF_XMIT_DST_RELEASE 0x20	/* dev_hard_start_xmit() is allowed to
>   					 * release skb->dst
>   					 */
> -#define IFF_DONT_BRIDGE 0x80		/* disallow bridging this ether dev */
> -#define IFF_DISABLE_NETPOLL	0x100	/* disable netpoll at run-time */
> -#define IFF_MACVLAN_PORT	0x200	/* device used as macvlan port */
> -#define IFF_BRIDGE_PORT	0x400		/* device used as bridge port */
> -#define IFF_OVS_DATAPATH	0x800	/* device used as Open vSwitch
> +#define IFF_DONT_BRIDGE 0x40		/* disallow bridging this ether dev */
> +#define IFF_DISABLE_NETPOLL	0x80	/* disable netpoll at run-time */
> +#define IFF_MACVLAN_PORT	0x100	/* device used as macvlan port */
> +#define IFF_BRIDGE_PORT	0x200		/* device used as bridge port */
> +#define IFF_OVS_DATAPATH	0x400	/* device used as Open vSwitch
>   					 * datapath port */
>
>   #define IF_GET_IFACE	0x0001		/* for querying only */


^ permalink raw reply

* Re: [RFC PATCH] net/core: fix skb handling on netif serves for both bridge and vlan
From: Jiri Pirko @ 2011-03-05 14:25 UTC (permalink / raw)
  To: Nicolas de Pesloüan
  Cc: Xiaotian Feng, netdev, linux-kernel, David S. Miller,
	Eric Dumazet, Tom Herbert
In-Reply-To: <4D72406B.6090504@gmail.com>

Sat, Mar 05, 2011 at 02:53:47PM CET, nicolas.2p.debian@gmail.com wrote:
>Le 05/03/2011 11:36, Jiri Pirko a écrit :
>>Thu, Mar 03, 2011 at 11:55:13AM CET, dfeng@redhat.com wrote:
>>>Consider network topology as follows:
>>>
>>>eth0  eth1
>>>|_____|
>>>    |
>>>  bond0 --- br0
>>>    |
>>>  vlan0 --- br1
>>>
>>>bond0 serves for both br0 and vlan0, if a vlan tagged packet was sent
>>>to br1 through bond0, bridge handling code is seeing the packet on bond0
>>>and handing it off to my "legacy" bridge before vlan_tx_tag_present
>>>and vlan_hwaccel_do_receive even haven't a chance to look at it.
>>>
>>>Moving the vlan_tx_tag_present before bridge/macvlan handling code could
>>>cure this.
>>
>>Wouldn't this break "eth0 - br0 - br0.5"?
>
>I think it would. One more reason to build a single interface stacking framework...

I plan to deal with vlans later. This kind of topology would not be
possible after that :)
>
>	Nicolas.

^ permalink raw reply

* Re: [patch net-next-2.6 2/8] bonding: register slave pointer for rx_handler
From: Jiri Pirko @ 2011-03-05 14:27 UTC (permalink / raw)
  To: Nicolas de Pesloüan
  Cc: netdev, davem, shemminger, kaber, fubar, eric.dumazet, andy
In-Reply-To: <4D724382.7010705@gmail.com>

Sat, Mar 05, 2011 at 03:06:58PM CET, nicolas.2p.debian@gmail.com wrote:
>Le 05/03/2011 11:29, Jiri Pirko a écrit :
>>Register slave pointer as rx_handler data. That would eventually prevent
>>need to loop over slave devices to find the right slave.
>>
>>Use synchronize_net to ensure that bond_handle_frame does not get slave
>>structure freed when working with that.
>>
>>Signed-off-by: Jiri Pirko<jpirko@redhat.com>
>>---
>>  drivers/net/bonding/bond_main.c |   17 +++++++++++------
>>  drivers/net/bonding/bonding.h   |    3 +++
>>  2 files changed, 14 insertions(+), 6 deletions(-)
>>
>>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>>index 0592e6d..1c19368 100644
>>--- a/drivers/net/bonding/bond_main.c
>>+++ b/drivers/net/bonding/bond_main.c
>>@@ -1495,21 +1495,22 @@ static bool bond_should_deliver_exact_match(struct sk_buff *skb,
>>
>>  static struct sk_buff *bond_handle_frame(struct sk_buff *skb)
>>  {
>>-	struct net_device *slave_dev;
>>+	struct slave *slave;
>>  	struct net_device *bond_dev;
>>
>>  	skb = skb_share_check(skb, GFP_ATOMIC);
>>  	if (unlikely(!skb))
>>  		return NULL;
>>-	slave_dev = skb->dev;
>>-	bond_dev = ACCESS_ONCE(slave_dev->master);
>>+
>>+	slave = bond_slave_get_rcu(skb->dev);
>>+	bond_dev = ACCESS_ONCE(slave->dev->master);
>>  	if (unlikely(!bond_dev))
>>  		return skb;
>>
>>  	if (bond_dev->priv_flags&  IFF_MASTER_ARPMON)
>>-		slave_dev->last_rx = jiffies;
>>+		slave->dev->last_rx = jiffies;
>>
>>-	if (bond_should_deliver_exact_match(skb, slave_dev, bond_dev)) {
>>+	if (bond_should_deliver_exact_match(skb, slave->dev, bond_dev)) {
>>  		skb->deliver_no_wcard = 1;
>>  		return skb;
>>  	}
>
>Up to this point, it looks like cleanup, and unrelated to the title and description of the patch.

Nope - these are changes connected to the ability to get slave struct
via bond_slave_get_rcu. Very related.

>
>Anyway, the cleanup looks good to me. Should just be in a separate patch.
>
>>@@ -1703,7 +1704,8 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
>>  		pr_debug("Error %d calling netdev_set_bond_master\n", res);
>>  		goto err_restore_mac;
>>  	}
>>-	res = netdev_rx_handler_register(slave_dev, bond_handle_frame, NULL);
>>+	res = netdev_rx_handler_register(slave_dev, bond_handle_frame,
>>+					 new_slave);
>
>And using rx_handler data for this purpose sounds good to me.
>
>Reviewed-by: Nicolas de Pesloüan <nicolas.2p.debian@free.fr>
>
>>  	if (res) {
>>  		pr_debug("Error %d calling netdev_rx_handler_register\n", res);
>>  		goto err_unset_master;
>>@@ -1925,6 +1927,7 @@ err_close:
>>
>>  err_unreg_rxhandler:
>>  	netdev_rx_handler_unregister(slave_dev);
>>+	synchronize_net();
>>
>>  err_unset_master:
>>  	netdev_set_bond_master(slave_dev, NULL);
>>@@ -2108,6 +2111,7 @@ int bond_release(struct net_device *bond_dev, struct net_device *slave_dev)
>>  	}
>>
>>  	netdev_rx_handler_unregister(slave_dev);
>>+	synchronize_net();
>>  	netdev_set_bond_master(slave_dev, NULL);
>>
>>  	slave_disable_netpoll(slave);
>>@@ -2222,6 +2226,7 @@ static int bond_release_all(struct net_device *bond_dev)
>>  		}
>>
>>  		netdev_rx_handler_unregister(slave_dev);
>>+		synchronize_net();
>>  		netdev_set_bond_master(slave_dev, NULL);
>>
>>  		slave_disable_netpoll(slave);
>>diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
>>index ff4e269..1aac5cd 100644
>>--- a/drivers/net/bonding/bonding.h
>>+++ b/drivers/net/bonding/bonding.h
>>@@ -264,6 +264,9 @@ struct bonding {
>>  #endif /* CONFIG_DEBUG_FS */
>>  };
>>
>>+#define bond_slave_get_rcu(dev) \
>>+	((struct slave *) rcu_dereference(dev->rx_handler_data))
>>+
>>  /**
>>   * Returns NULL if the net_device does not belong to any of the bond's slaves
>>   *
>

^ permalink raw reply

* Re: [patch net-next-2.6 6/8] bonding: move processing of recv handlers into handle_frame()
From: Nicolas de Pesloüan @ 2011-03-05 14:33 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: netdev, davem, shemminger, kaber, fubar, eric.dumazet, andy
In-Reply-To: <1299320969-7951-7-git-send-email-jpirko@redhat.com>

Le 05/03/2011 11:29, Jiri Pirko a écrit :
> Since now when bonding uses rx_handler, all traffic going into bond
> device goes thru bond_handle_frame. So there's no need to go back into
> bonding code later via ptype handlers. This patch converts
> original ptype handlers into "bonding receive probes". These functions
> are called from bond_handle_frame and they are registered per-mode.

Does this still support having the arp_ip_target on a vlan?

(eth0 -> bond0 -> bond0.100, with arp_ip_target only reachable through bond0.100).

>
> Signed-off-by: Jiri Pirko<jpirko@redhat.com>
> ---
>   drivers/net/bonding/bond_3ad.c   |   29 ++--------
>   drivers/net/bonding/bond_3ad.h   |    4 +-
>   drivers/net/bonding/bond_alb.c   |   46 ++++-----------
>   drivers/net/bonding/bond_alb.h   |    1 -
>   drivers/net/bonding/bond_main.c  |  121 +++++++------------------------------
>   drivers/net/bonding/bond_sysfs.c |    6 --
>   drivers/net/bonding/bonding.h    |    4 +-
>   7 files changed, 43 insertions(+), 168 deletions(-)
>
> diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
> index 047af0b..194aaf7 100644
> --- a/drivers/net/bonding/bond_3ad.c
> +++ b/drivers/net/bonding/bond_3ad.c
> @@ -2461,35 +2461,16 @@ out:
>   	return NETDEV_TX_OK;
>   }
>
> -int bond_3ad_lacpdu_recv(struct sk_buff *skb, struct net_device *dev, struct packet_type* ptype, struct net_device *orig_dev)
> +void bond_3ad_lacpdu_recv(struct sk_buff *skb, struct bonding *bond,
> +			  struct slave *slave)
>   {
> -	struct bonding *bond = netdev_priv(dev);
> -	struct slave *slave = NULL;
> -	int ret = NET_RX_DROP;
> -
> -	if (!(dev->flags&  IFF_MASTER))
> -		goto out;
> -
> -	skb = skb_share_check(skb, GFP_ATOMIC);
> -	if (!skb)
> -		goto out;
> +	if (skb->protocol != PKT_TYPE_LACPDU)
> +		return;
>
>   	if (!pskb_may_pull(skb, sizeof(struct lacpdu)))
> -		goto out;
> +		return;
>
>   	read_lock(&bond->lock);
> -	slave = bond_get_slave_by_dev(netdev_priv(dev), orig_dev);
> -	if (!slave)
> -		goto out_unlock;
> -
>   	bond_3ad_rx_indication((struct lacpdu *) skb->data, slave, skb->len);
> -
> -	ret = NET_RX_SUCCESS;
> -
> -out_unlock:
>   	read_unlock(&bond->lock);
> -out:
> -	dev_kfree_skb(skb);
> -
> -	return ret;
>   }
> diff --git a/drivers/net/bonding/bond_3ad.h b/drivers/net/bonding/bond_3ad.h
> index 2c46a154..56a88ff 100644
> --- a/drivers/net/bonding/bond_3ad.h
> +++ b/drivers/net/bonding/bond_3ad.h
> @@ -258,7 +258,6 @@ struct ad_bond_info {
>   				 * requested
>   				 */
>   	struct timer_list ad_timer;
> -	struct packet_type ad_pkt_type;
>   };
>
>   struct ad_slave_info {
> @@ -279,7 +278,8 @@ void bond_3ad_adapter_duplex_changed(struct slave *slave);
>   void bond_3ad_handle_link_change(struct slave *slave, char link);
>   int  bond_3ad_get_active_agg_info(struct bonding *bond, struct ad_info *ad_info);
>   int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev);
> -int bond_3ad_lacpdu_recv(struct sk_buff *skb, struct net_device *dev, struct packet_type* ptype, struct net_device *orig_dev);
> +void bond_3ad_lacpdu_recv(struct sk_buff *skb, struct bonding *bond,
> +			  struct slave *slave);
>   int bond_3ad_set_carrier(struct bonding *bond);
>   #endif //__BOND_3AD_H__
>
> diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
> index 9bc5de3..96d28f9 100644
> --- a/drivers/net/bonding/bond_alb.c
> +++ b/drivers/net/bonding/bond_alb.c
> @@ -308,49 +308,33 @@ static void rlb_update_entry_from_arp(struct bonding *bond, struct arp_pkt *arp)
>   	_unlock_rx_hashtbl(bond);
>   }
>
> -static int rlb_arp_recv(struct sk_buff *skb, struct net_device *bond_dev, struct packet_type *ptype, struct net_device *orig_dev)
> +static void rlb_arp_recv(struct sk_buff *skb, struct bonding *bond,
> +			 struct slave *slave)
>   {
> -	struct bonding *bond;
> -	struct arp_pkt *arp = (struct arp_pkt *)skb->data;
> -	int res = NET_RX_DROP;
> +	struct arp_pkt *arp;
>
> -	while (bond_dev->priv_flags&  IFF_802_1Q_VLAN)
> -		bond_dev = vlan_dev_real_dev(bond_dev);
> -
> -	if (!(bond_dev->priv_flags&  IFF_BONDING) ||
> -	    !(bond_dev->flags&  IFF_MASTER))
> -		goto out;
> +	if (skb->protocol != cpu_to_be16(ETH_P_ARP))
> +		return;
>
> +	arp = (struct arp_pkt *) skb->data;
>   	if (!arp) {
>   		pr_debug("Packet has no ARP data\n");
> -		goto out;
> +		return;
>   	}
>
> -	skb = skb_share_check(skb, GFP_ATOMIC);
> -	if (!skb)
> -		goto out;
> -
> -	if (!pskb_may_pull(skb, arp_hdr_len(bond_dev)))
> -		goto out;
> +	if (!pskb_may_pull(skb, arp_hdr_len(bond->dev)))
> +		return;
>
>   	if (skb->len<  sizeof(struct arp_pkt)) {
>   		pr_debug("Packet is too small to be an ARP\n");
> -		goto out;
> +		return;
>   	}
>
>   	if (arp->op_code == htons(ARPOP_REPLY)) {
>   		/* update rx hash table for this ARP */
> -		bond = netdev_priv(bond_dev);
>   		rlb_update_entry_from_arp(bond, arp);
>   		pr_debug("Server received an ARP Reply from client\n");
>   	}
> -
> -	res = NET_RX_SUCCESS;
> -
> -out:
> -	dev_kfree_skb(skb);
> -
> -	return res;
>   }
>
>   /* Caller must hold bond lock for read */
> @@ -759,7 +743,6 @@ static void rlb_init_table_entry(struct rlb_client_info *entry)
>   static int rlb_initialize(struct bonding *bond)
>   {
>   	struct alb_bond_info *bond_info =&(BOND_ALB_INFO(bond));
> -	struct packet_type *pk_type =&(BOND_ALB_INFO(bond).rlb_pkt_type);
>   	struct rlb_client_info	*new_hashtbl;
>   	int size = RLB_HASH_TABLE_SIZE * sizeof(struct rlb_client_info);
>   	int i;
> @@ -784,13 +767,8 @@ static int rlb_initialize(struct bonding *bond)
>
>   	_unlock_rx_hashtbl(bond);
>
> -	/*initialize packet type*/
> -	pk_type->type = cpu_to_be16(ETH_P_ARP);
> -	pk_type->dev = bond->dev;
> -	pk_type->func = rlb_arp_recv;
> -
>   	/* register to receive ARPs */
> -	dev_add_pack(pk_type);
> +	bond->recv_probe = rlb_arp_recv;
>
>   	return 0;
>   }
> @@ -799,8 +777,6 @@ static void rlb_deinitialize(struct bonding *bond)
>   {
>   	struct alb_bond_info *bond_info =&(BOND_ALB_INFO(bond));
>
> -	dev_remove_pack(&(bond_info->rlb_pkt_type));
> -
>   	_lock_rx_hashtbl(bond);
>
>   	kfree(bond_info->rx_hashtbl);
> diff --git a/drivers/net/bonding/bond_alb.h b/drivers/net/bonding/bond_alb.h
> index 118c28a..6605e9e 100644
> --- a/drivers/net/bonding/bond_alb.h
> +++ b/drivers/net/bonding/bond_alb.h
> @@ -130,7 +130,6 @@ struct alb_bond_info {
>   	int			lp_counter;
>   	/* -------- rlb parameters -------- */
>   	int rlb_enabled;
> -	struct packet_type	rlb_pkt_type;
>   	struct rlb_client_info	*rx_hashtbl;	/* Receive hash table */
>   	spinlock_t		rx_hashtbl_lock;
>   	u32			rx_hashtbl_head;
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index dbe182c..8ba7faa 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -1467,27 +1467,17 @@ static void bond_setup_by_slave(struct net_device *bond_dev,
>   }
>
>   /* On bonding slaves other than the currently active slave, suppress
> - * duplicates except for 802.3ad ETH_P_SLOW, alb non-mcast/bcast, and
> - * ARP on active-backup slaves with arp_validate enabled.
> + * duplicates except for alb non-mcast/bcast.
>    */
>   static bool bond_should_deliver_exact_match(struct sk_buff *skb,
>   					    struct slave *slave,
>   					    struct bonding *bond)
>   {
>   	if (bond_is_slave_inactive(slave)) {
> -		if (slave_do_arp_validate(bond, slave)&&
> -		    skb->protocol == __cpu_to_be16(ETH_P_ARP))
> -			return false;
> -
>   		if (bond->params.mode == BOND_MODE_ALB&&
>   		skb->pkt_type != PACKET_BROADCAST&&
>   		skb->pkt_type != PACKET_MULTICAST)
> -				return false;
> -
> -		if (bond->params.mode == BOND_MODE_8023AD&&
> -		    skb->protocol == __cpu_to_be16(ETH_P_SLOW))
>   			return false;
> -
>   		return true;
>   	}
>   	return false;
> @@ -1513,6 +1503,15 @@ static struct sk_buff *bond_handle_frame(struct sk_buff *skb)
>   	if (bond->params.arp_interval)
>   		slave->dev->last_rx = jiffies;
>
> +	if (bond->recv_probe) {
> +		struct sk_buff *nskb = skb_clone(skb, GFP_ATOMIC);
> +
> +		if (likely(nskb)) {
> +			bond->recv_probe(nskb, bond, slave);
> +			dev_kfree_skb(nskb);
> +		}
> +	}
> +
>   	if (bond_should_deliver_exact_match(skb, slave, bond)) {
>   		skb->deliver_no_wcard = 1;
>   		return skb;
> @@ -2813,48 +2812,26 @@ static void bond_validate_arp(struct bonding *bond, struct slave *slave, __be32
>   	}
>   }
>
> -static int bond_arp_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *pt, struct net_device *orig_dev)
> +static void bond_arp_rcv(struct sk_buff *skb, struct bonding *bond,
> +			 struct slave *slave)
>   {
>   	struct arphdr *arp;
> -	struct slave *slave;
> -	struct bonding *bond;
>   	unsigned char *arp_ptr;
>   	__be32 sip, tip;
>
> -	if (dev->priv_flags&  IFF_802_1Q_VLAN) {
> -		/*
> -		 * When using VLANS and bonding, dev and oriv_dev may be
> -		 * incorrect if the physical interface supports VLAN
> -		 * acceleration.  With this change ARP validation now
> -		 * works for hosts only reachable on the VLAN interface.
> -		 */
> -		dev = vlan_dev_real_dev(dev);
> -		orig_dev = dev_get_by_index_rcu(dev_net(skb->dev),skb->skb_iif);
> -	}
> -
> -	if (!(dev->priv_flags&  IFF_BONDING) || !(dev->flags&  IFF_MASTER))
> -		goto out;
> +	if (skb->protocol != __cpu_to_be16(ETH_P_ARP))
> +		return;

What happens if the skb that hold the ARP request/reply is vlan tagged? The ARP ptype_base handler 
for bonding used to be called after vlan processing.

> -	bond = netdev_priv(dev);
>   	read_lock(&bond->lock);
>
> -	pr_debug("bond_arp_rcv: bond %s skb->dev %s orig_dev %s\n",
> -		 bond->dev->name, skb->dev ? skb->dev->name : "NULL",
> -		 orig_dev ? orig_dev->name : "NULL");
> +	pr_debug("bond_arp_rcv: bond %s skb->dev %s\n",
> +		 bond->dev->name, skb->dev->name);
>
> -	slave = bond_get_slave_by_dev(bond, orig_dev);
> -	if (!slave || !slave_do_arp_validate(bond, slave))
> -		goto out_unlock;
> -
> -	skb = skb_share_check(skb, GFP_ATOMIC);
> -	if (!skb)
> -		goto out_unlock;
> -
> -	if (!pskb_may_pull(skb, arp_hdr_len(dev)))
> +	if (!pskb_may_pull(skb, arp_hdr_len(bond->dev)))
>   		goto out_unlock;
>
>   	arp = arp_hdr(skb);
> -	if (arp->ar_hln != dev->addr_len ||
> +	if (arp->ar_hln != bond->dev->addr_len ||
>   	    skb->pkt_type == PACKET_OTHERHOST ||
>   	    skb->pkt_type == PACKET_LOOPBACK ||
>   	    arp->ar_hrd != htons(ARPHRD_ETHER) ||
> @@ -2863,9 +2840,9 @@ static int bond_arp_rcv(struct sk_buff *skb, struct net_device *dev, struct pack
>   		goto out_unlock;
>
>   	arp_ptr = (unsigned char *)(arp + 1);
> -	arp_ptr += dev->addr_len;
> +	arp_ptr += bond->dev->addr_len;
>   	memcpy(&sip, arp_ptr, 4);
> -	arp_ptr += 4 + dev->addr_len;
> +	arp_ptr += 4 + bond->dev->addr_len;
>   	memcpy(&tip, arp_ptr, 4);
>
>   	pr_debug("bond_arp_rcv: %s %s/%d av %d sv %d sip %pI4 tip %pI4\n",
> @@ -2888,9 +2865,6 @@ static int bond_arp_rcv(struct sk_buff *skb, struct net_device *dev, struct pack
>
>   out_unlock:
>   	read_unlock(&bond->lock);
> -out:
> -	dev_kfree_skb(skb);
> -	return NET_RX_SUCCESS;
>   }
>
>   /*
> @@ -3782,48 +3756,6 @@ static struct notifier_block bond_inetaddr_notifier = {
>   	.notifier_call = bond_inetaddr_event,
>   };
>
> -/*-------------------------- Packet type handling ---------------------------*/
> -
> -/* register to receive lacpdus on a bond */
> -static void bond_register_lacpdu(struct bonding *bond)
> -{
> -	struct packet_type *pk_type =&(BOND_AD_INFO(bond).ad_pkt_type);
> -
> -	/* initialize packet type */
> -	pk_type->type = PKT_TYPE_LACPDU;
> -	pk_type->dev = bond->dev;
> -	pk_type->func = bond_3ad_lacpdu_recv;
> -
> -	dev_add_pack(pk_type);
> -}
> -
> -/* unregister to receive lacpdus on a bond */
> -static void bond_unregister_lacpdu(struct bonding *bond)
> -{
> -	dev_remove_pack(&(BOND_AD_INFO(bond).ad_pkt_type));
> -}
> -
> -void bond_register_arp(struct bonding *bond)
> -{
> -	struct packet_type *pt =&bond->arp_mon_pt;
> -
> -	if (pt->type)
> -		return;
> -
> -	pt->type = htons(ETH_P_ARP);
> -	pt->dev = bond->dev;
> -	pt->func = bond_arp_rcv;
> -	dev_add_pack(pt);
> -}
> -
> -void bond_unregister_arp(struct bonding *bond)
> -{
> -	struct packet_type *pt =&bond->arp_mon_pt;
> -
> -	dev_remove_pack(pt);
> -	pt->type = 0;
> -}
> -
>   /*---------------------------- Hashing Policies -----------------------------*/
>
>   /*
> @@ -3917,14 +3849,14 @@ static int bond_open(struct net_device *bond_dev)
>
>   		queue_delayed_work(bond->wq,&bond->arp_work, 0);
>   		if (bond->params.arp_validate)
> -			bond_register_arp(bond);
> +			bond->recv_probe = bond_arp_rcv;
>   	}
>
>   	if (bond->params.mode == BOND_MODE_8023AD) {
>   		INIT_DELAYED_WORK(&bond->ad_work, bond_3ad_state_machine_handler);
>   		queue_delayed_work(bond->wq,&bond->ad_work, 0);
>   		/* register to receive LACPDUs */
> -		bond_register_lacpdu(bond);
> +		bond->recv_probe = bond_3ad_lacpdu_recv;
>   		bond_3ad_initiate_agg_selection(bond, 1);
>   	}
>
> @@ -3935,14 +3867,6 @@ static int bond_close(struct net_device *bond_dev)
>   {
>   	struct bonding *bond = netdev_priv(bond_dev);
>
> -	if (bond->params.mode == BOND_MODE_8023AD) {
> -		/* Unregister the receive of LACPDUs */
> -		bond_unregister_lacpdu(bond);
> -	}
> -
> -	if (bond->params.arp_validate)
> -		bond_unregister_arp(bond);
> -
>   	write_lock_bh(&bond->lock);
>
>   	bond->send_grat_arp = 0;
> @@ -3982,6 +3906,7 @@ static int bond_close(struct net_device *bond_dev)
>   		 */
>   		bond_alb_deinitialize(bond);
>   	}
> +	bond->recv_probe = NULL;
>
>   	return 0;
>   }
> diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
> index 5161183..6804efe 100644
> --- a/drivers/net/bonding/bond_sysfs.c
> +++ b/drivers/net/bonding/bond_sysfs.c
> @@ -419,11 +419,6 @@ static ssize_t bonding_store_arp_validate(struct device *d,
>   		bond->dev->name, arp_validate_tbl[new_value].modename,
>   		new_value);
>
> -	if (!bond->params.arp_validate&&  new_value)
> -		bond_register_arp(bond);
> -	else if (bond->params.arp_validate&&  !new_value)
> -		bond_unregister_arp(bond);
> -
>   	bond->params.arp_validate = new_value;
>
>   	return count;
> @@ -998,7 +993,6 @@ static ssize_t bonding_store_miimon(struct device *d,
>   				bond->dev->name);
>   			bond->params.arp_interval = 0;
>   			if (bond->params.arp_validate) {
> -				bond_unregister_arp(bond);
>   				bond->params.arp_validate =
>   					BOND_ARP_VALIDATE_NONE;
>   			}
> diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
> index 8f78166..75a1308 100644
> --- a/drivers/net/bonding/bonding.h
> +++ b/drivers/net/bonding/bonding.h
> @@ -228,6 +228,8 @@ struct bonding {
>   	struct   slave *primary_slave;
>   	bool     force_primary;
>   	s32      slave_cnt; /* never change this value outside the attach/detach wrappers */
> +	void     (*recv_probe)(struct sk_buff *, struct bonding *,
> +			       struct slave *);
>   	rwlock_t lock;
>   	rwlock_t curr_slave_lock;
>   	s8       kill_timers;
> @@ -406,8 +408,6 @@ void bond_set_mode_ops(struct bonding *bond, int mode);
>   int bond_parse_parm(const char *mode_arg, const struct bond_parm_tbl *tbl);
>   void bond_select_active_slave(struct bonding *bond);
>   void bond_change_active_slave(struct bonding *bond, struct slave *new_active);
> -void bond_register_arp(struct bonding *);
> -void bond_unregister_arp(struct bonding *);
>   void bond_create_debugfs(void);
>   void bond_destroy_debugfs(void);
>   void bond_debug_register(struct bonding *bond);


^ permalink raw reply

* Re: [patch net-next-2.6 3/8] net: get rid of multiple bond-related netdevice->priv_flags
From: Ben Hutchings @ 2011-03-05 14:37 UTC (permalink / raw)
  To: Nicolas de Pesloüan
  Cc: Jiri Pirko, netdev, davem, shemminger, kaber, fubar, eric.dumazet,
	andy
In-Reply-To: <4D72453F.10909@gmail.com>

On Sat, 2011-03-05 at 15:14 +0100, Nicolas de Pesloüan wrote:
> Le 05/03/2011 11:29, Jiri Pirko a écrit :
[...]
> > --- a/include/linux/if.h
> > +++ b/include/linux/if.h
> > @@ -60,21 +60,17 @@
> >   #define IFF_802_1Q_VLAN 0x1             /* 802.1Q VLAN device.          */
> >   #define IFF_EBRIDGE	0x2		/* Ethernet bridging device.	*/
> >   #define IFF_SLAVE_INACTIVE	0x4	/* bonding slave not the curr. active */
> > -#define IFF_MASTER_8023AD	0x8	/* bonding master, 802.3ad. 	*/
> > -#define IFF_MASTER_ALB	0x10		/* bonding master, balance-alb.	*/
> > -#define IFF_BONDING	0x20		/* bonding master or slave	*/
> > -#define IFF_SLAVE_NEEDARP 0x40		/* need ARPs for validation	*/
> > -#define IFF_ISATAP	0x80		/* ISATAP interface (RFC4214)	*/
> > -#define IFF_MASTER_ARPMON 0x100		/* bonding master, ARP mon in use */
> > -#define IFF_WAN_HDLC	0x200		/* WAN HDLC device		*/
> > -#define IFF_XMIT_DST_RELEASE 0x400	/* dev_hard_start_xmit() is allowed to
> > +#define IFF_BONDING	0x8		/* bonding master or slave	*/
> > +#define IFF_ISATAP	0x10		/* ISATAP interface (RFC4214)	*/
> > +#define IFF_WAN_HDLC	0x20		/* WAN HDLC device		*/
> > +#define IFF_XMIT_DST_RELEASE 0x40	/* dev_hard_start_xmit() is allowed to
> >   					 * release skb->dst
> >   					 */
> 
> Why did you changed those values? Aren't those exposed to userland? Just removing the unused one 
> sounds good to me.
[...]

Look just one line further up:

/* Private (from user) interface flags (netdevice->priv_flags). */

Maybe they should have a prefix IFPF_ instead of IFF_ though.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.


^ permalink raw reply

* Re: [patch net-next-2.6 2/8] bonding: register slave pointer for rx_handler
From: Nicolas de Pesloüan @ 2011-03-05 14:38 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: netdev, davem, shemminger, kaber, fubar, eric.dumazet, andy
In-Reply-To: <20110305142732.GB8573@psychotron.redhat.com>

Le 05/03/2011 15:27, Jiri Pirko a écrit :
> Sat, Mar 05, 2011 at 03:06:58PM CET, nicolas.2p.debian@gmail.com wrote:
>> Le 05/03/2011 11:29, Jiri Pirko a écrit :
>>> Register slave pointer as rx_handler data. That would eventually prevent
>>> need to loop over slave devices to find the right slave.
>>>
>>> Use synchronize_net to ensure that bond_handle_frame does not get slave
>>> structure freed when working with that.
>>>
>>> Signed-off-by: Jiri Pirko<jpirko@redhat.com>
>>> ---
>>>   drivers/net/bonding/bond_main.c |   17 +++++++++++------
>>>   drivers/net/bonding/bonding.h   |    3 +++
>>>   2 files changed, 14 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>>> index 0592e6d..1c19368 100644
>>> --- a/drivers/net/bonding/bond_main.c
>>> +++ b/drivers/net/bonding/bond_main.c
>>> @@ -1495,21 +1495,22 @@ static bool bond_should_deliver_exact_match(struct sk_buff *skb,
>>>
>>>   static struct sk_buff *bond_handle_frame(struct sk_buff *skb)
>>>   {
>>> -	struct net_device *slave_dev;
>>> +	struct slave *slave;
>>>   	struct net_device *bond_dev;
>>>
>>>   	skb = skb_share_check(skb, GFP_ATOMIC);
>>>   	if (unlikely(!skb))
>>>   		return NULL;
>>> -	slave_dev = skb->dev;
>>> -	bond_dev = ACCESS_ONCE(slave_dev->master);
>>> +
>>> +	slave = bond_slave_get_rcu(skb->dev);
>>> +	bond_dev = ACCESS_ONCE(slave->dev->master);
>>>   	if (unlikely(!bond_dev))
>>>   		return skb;
>>>
>>>   	if (bond_dev->priv_flags&   IFF_MASTER_ARPMON)
>>> -		slave_dev->last_rx = jiffies;
>>> +		slave->dev->last_rx = jiffies;
>>>
>>> -	if (bond_should_deliver_exact_match(skb, slave_dev, bond_dev)) {
>>> +	if (bond_should_deliver_exact_match(skb, slave->dev, bond_dev)) {
>>>   		skb->deliver_no_wcard = 1;
>>>   		return skb;
>>>   	}
>>
>> Up to this point, it looks like cleanup, and unrelated to the title and description of the patch.
>
> Nope - these are changes connected to the ability to get slave struct
> via bond_slave_get_rcu. Very related.

Agreed.

>>
>> Anyway, the cleanup looks good to me. Should just be in a separate patch.
>>
>>> @@ -1703,7 +1704,8 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
>>>   		pr_debug("Error %d calling netdev_set_bond_master\n", res);
>>>   		goto err_restore_mac;
>>>   	}
>>> -	res = netdev_rx_handler_register(slave_dev, bond_handle_frame, NULL);
>>> +	res = netdev_rx_handler_register(slave_dev, bond_handle_frame,
>>> +					 new_slave);
>>
>> And using rx_handler data for this purpose sounds good to me.
>>
>> Reviewed-by: Nicolas de Pesloüan<nicolas.2p.debian@free.fr>
>>
>>>   	if (res) {
>>>   		pr_debug("Error %d calling netdev_rx_handler_register\n", res);
>>>   		goto err_unset_master;
>>> @@ -1925,6 +1927,7 @@ err_close:
>>>
>>>   err_unreg_rxhandler:
>>>   	netdev_rx_handler_unregister(slave_dev);
>>> +	synchronize_net();
>>>
>>>   err_unset_master:
>>>   	netdev_set_bond_master(slave_dev, NULL);
>>> @@ -2108,6 +2111,7 @@ int bond_release(struct net_device *bond_dev, struct net_device *slave_dev)
>>>   	}
>>>
>>>   	netdev_rx_handler_unregister(slave_dev);
>>> +	synchronize_net();
>>>   	netdev_set_bond_master(slave_dev, NULL);
>>>
>>>   	slave_disable_netpoll(slave);
>>> @@ -2222,6 +2226,7 @@ static int bond_release_all(struct net_device *bond_dev)
>>>   		}
>>>
>>>   		netdev_rx_handler_unregister(slave_dev);
>>> +		synchronize_net();
>>>   		netdev_set_bond_master(slave_dev, NULL);
>>>
>>>   		slave_disable_netpoll(slave);
>>> diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
>>> index ff4e269..1aac5cd 100644
>>> --- a/drivers/net/bonding/bonding.h
>>> +++ b/drivers/net/bonding/bonding.h
>>> @@ -264,6 +264,9 @@ struct bonding {
>>>   #endif /* CONFIG_DEBUG_FS */
>>>   };
>>>
>>> +#define bond_slave_get_rcu(dev) \
>>> +	((struct slave *) rcu_dereference(dev->rx_handler_data))
>>> +
>>>   /**
>>>    * Returns NULL if the net_device does not belong to any of the bond's slaves
>>>    *
>>
>


^ permalink raw reply

* Re: [patch net-next-2.6 6/8] bonding: move processing of recv handlers into handle_frame()
From: Jiri Pirko @ 2011-03-05 14:43 UTC (permalink / raw)
  To: Nicolas de Pesloüan
  Cc: netdev, davem, shemminger, kaber, fubar, eric.dumazet, andy
In-Reply-To: <4D7249BA.8030401@gmail.com>

Sat, Mar 05, 2011 at 03:33:30PM CET, nicolas.2p.debian@gmail.com wrote:
>Le 05/03/2011 11:29, Jiri Pirko a écrit :
>>Since now when bonding uses rx_handler, all traffic going into bond
>>device goes thru bond_handle_frame. So there's no need to go back into
>>bonding code later via ptype handlers. This patch converts
>>original ptype handlers into "bonding receive probes". These functions
>>are called from bond_handle_frame and they are registered per-mode.
>
>Does this still support having the arp_ip_target on a vlan?
>
>(eth0 -> bond0 -> bond0.100, with arp_ip_target only reachable through bond0.100).

This case is still covered with vlan_on_bond_hook
eth0->
	bond_handle_frame
bond0->
	vlan_hwaccel_do_receive
bond0.5->
	vlan_on_bond_hook -> reinject into bond0
				-> bond_handle_frame (here it is processed)

>
>>
>>Signed-off-by: Jiri Pirko<jpirko@redhat.com>
>>---
>>  drivers/net/bonding/bond_3ad.c   |   29 ++--------
>>  drivers/net/bonding/bond_3ad.h   |    4 +-
>>  drivers/net/bonding/bond_alb.c   |   46 ++++-----------
>>  drivers/net/bonding/bond_alb.h   |    1 -
>>  drivers/net/bonding/bond_main.c  |  121 +++++++------------------------------
>>  drivers/net/bonding/bond_sysfs.c |    6 --
>>  drivers/net/bonding/bonding.h    |    4 +-
>>  7 files changed, 43 insertions(+), 168 deletions(-)
>>
>>diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
>>index 047af0b..194aaf7 100644
>>--- a/drivers/net/bonding/bond_3ad.c
>>+++ b/drivers/net/bonding/bond_3ad.c
>>@@ -2461,35 +2461,16 @@ out:
>>  	return NETDEV_TX_OK;
>>  }
>>
>>-int bond_3ad_lacpdu_recv(struct sk_buff *skb, struct net_device *dev, struct packet_type* ptype, struct net_device *orig_dev)
>>+void bond_3ad_lacpdu_recv(struct sk_buff *skb, struct bonding *bond,
>>+			  struct slave *slave)
>>  {
>>-	struct bonding *bond = netdev_priv(dev);
>>-	struct slave *slave = NULL;
>>-	int ret = NET_RX_DROP;
>>-
>>-	if (!(dev->flags&  IFF_MASTER))
>>-		goto out;
>>-
>>-	skb = skb_share_check(skb, GFP_ATOMIC);
>>-	if (!skb)
>>-		goto out;
>>+	if (skb->protocol != PKT_TYPE_LACPDU)
>>+		return;
>>
>>  	if (!pskb_may_pull(skb, sizeof(struct lacpdu)))
>>-		goto out;
>>+		return;
>>
>>  	read_lock(&bond->lock);
>>-	slave = bond_get_slave_by_dev(netdev_priv(dev), orig_dev);
>>-	if (!slave)
>>-		goto out_unlock;
>>-
>>  	bond_3ad_rx_indication((struct lacpdu *) skb->data, slave, skb->len);
>>-
>>-	ret = NET_RX_SUCCESS;
>>-
>>-out_unlock:
>>  	read_unlock(&bond->lock);
>>-out:
>>-	dev_kfree_skb(skb);
>>-
>>-	return ret;
>>  }
>>diff --git a/drivers/net/bonding/bond_3ad.h b/drivers/net/bonding/bond_3ad.h
>>index 2c46a154..56a88ff 100644
>>--- a/drivers/net/bonding/bond_3ad.h
>>+++ b/drivers/net/bonding/bond_3ad.h
>>@@ -258,7 +258,6 @@ struct ad_bond_info {
>>  				 * requested
>>  				 */
>>  	struct timer_list ad_timer;
>>-	struct packet_type ad_pkt_type;
>>  };
>>
>>  struct ad_slave_info {
>>@@ -279,7 +278,8 @@ void bond_3ad_adapter_duplex_changed(struct slave *slave);
>>  void bond_3ad_handle_link_change(struct slave *slave, char link);
>>  int  bond_3ad_get_active_agg_info(struct bonding *bond, struct ad_info *ad_info);
>>  int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev);
>>-int bond_3ad_lacpdu_recv(struct sk_buff *skb, struct net_device *dev, struct packet_type* ptype, struct net_device *orig_dev);
>>+void bond_3ad_lacpdu_recv(struct sk_buff *skb, struct bonding *bond,
>>+			  struct slave *slave);
>>  int bond_3ad_set_carrier(struct bonding *bond);
>>  #endif //__BOND_3AD_H__
>>
>>diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
>>index 9bc5de3..96d28f9 100644
>>--- a/drivers/net/bonding/bond_alb.c
>>+++ b/drivers/net/bonding/bond_alb.c
>>@@ -308,49 +308,33 @@ static void rlb_update_entry_from_arp(struct bonding *bond, struct arp_pkt *arp)
>>  	_unlock_rx_hashtbl(bond);
>>  }
>>
>>-static int rlb_arp_recv(struct sk_buff *skb, struct net_device *bond_dev, struct packet_type *ptype, struct net_device *orig_dev)
>>+static void rlb_arp_recv(struct sk_buff *skb, struct bonding *bond,
>>+			 struct slave *slave)
>>  {
>>-	struct bonding *bond;
>>-	struct arp_pkt *arp = (struct arp_pkt *)skb->data;
>>-	int res = NET_RX_DROP;
>>+	struct arp_pkt *arp;
>>
>>-	while (bond_dev->priv_flags&  IFF_802_1Q_VLAN)
>>-		bond_dev = vlan_dev_real_dev(bond_dev);
>>-
>>-	if (!(bond_dev->priv_flags&  IFF_BONDING) ||
>>-	    !(bond_dev->flags&  IFF_MASTER))
>>-		goto out;
>>+	if (skb->protocol != cpu_to_be16(ETH_P_ARP))
>>+		return;
>>
>>+	arp = (struct arp_pkt *) skb->data;
>>  	if (!arp) {
>>  		pr_debug("Packet has no ARP data\n");
>>-		goto out;
>>+		return;
>>  	}
>>
>>-	skb = skb_share_check(skb, GFP_ATOMIC);
>>-	if (!skb)
>>-		goto out;
>>-
>>-	if (!pskb_may_pull(skb, arp_hdr_len(bond_dev)))
>>-		goto out;
>>+	if (!pskb_may_pull(skb, arp_hdr_len(bond->dev)))
>>+		return;
>>
>>  	if (skb->len<  sizeof(struct arp_pkt)) {
>>  		pr_debug("Packet is too small to be an ARP\n");
>>-		goto out;
>>+		return;
>>  	}
>>
>>  	if (arp->op_code == htons(ARPOP_REPLY)) {
>>  		/* update rx hash table for this ARP */
>>-		bond = netdev_priv(bond_dev);
>>  		rlb_update_entry_from_arp(bond, arp);
>>  		pr_debug("Server received an ARP Reply from client\n");
>>  	}
>>-
>>-	res = NET_RX_SUCCESS;
>>-
>>-out:
>>-	dev_kfree_skb(skb);
>>-
>>-	return res;
>>  }
>>
>>  /* Caller must hold bond lock for read */
>>@@ -759,7 +743,6 @@ static void rlb_init_table_entry(struct rlb_client_info *entry)
>>  static int rlb_initialize(struct bonding *bond)
>>  {
>>  	struct alb_bond_info *bond_info =&(BOND_ALB_INFO(bond));
>>-	struct packet_type *pk_type =&(BOND_ALB_INFO(bond).rlb_pkt_type);
>>  	struct rlb_client_info	*new_hashtbl;
>>  	int size = RLB_HASH_TABLE_SIZE * sizeof(struct rlb_client_info);
>>  	int i;
>>@@ -784,13 +767,8 @@ static int rlb_initialize(struct bonding *bond)
>>
>>  	_unlock_rx_hashtbl(bond);
>>
>>-	/*initialize packet type*/
>>-	pk_type->type = cpu_to_be16(ETH_P_ARP);
>>-	pk_type->dev = bond->dev;
>>-	pk_type->func = rlb_arp_recv;
>>-
>>  	/* register to receive ARPs */
>>-	dev_add_pack(pk_type);
>>+	bond->recv_probe = rlb_arp_recv;
>>
>>  	return 0;
>>  }
>>@@ -799,8 +777,6 @@ static void rlb_deinitialize(struct bonding *bond)
>>  {
>>  	struct alb_bond_info *bond_info =&(BOND_ALB_INFO(bond));
>>
>>-	dev_remove_pack(&(bond_info->rlb_pkt_type));
>>-
>>  	_lock_rx_hashtbl(bond);
>>
>>  	kfree(bond_info->rx_hashtbl);
>>diff --git a/drivers/net/bonding/bond_alb.h b/drivers/net/bonding/bond_alb.h
>>index 118c28a..6605e9e 100644
>>--- a/drivers/net/bonding/bond_alb.h
>>+++ b/drivers/net/bonding/bond_alb.h
>>@@ -130,7 +130,6 @@ struct alb_bond_info {
>>  	int			lp_counter;
>>  	/* -------- rlb parameters -------- */
>>  	int rlb_enabled;
>>-	struct packet_type	rlb_pkt_type;
>>  	struct rlb_client_info	*rx_hashtbl;	/* Receive hash table */
>>  	spinlock_t		rx_hashtbl_lock;
>>  	u32			rx_hashtbl_head;
>>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>>index dbe182c..8ba7faa 100644
>>--- a/drivers/net/bonding/bond_main.c
>>+++ b/drivers/net/bonding/bond_main.c
>>@@ -1467,27 +1467,17 @@ static void bond_setup_by_slave(struct net_device *bond_dev,
>>  }
>>
>>  /* On bonding slaves other than the currently active slave, suppress
>>- * duplicates except for 802.3ad ETH_P_SLOW, alb non-mcast/bcast, and
>>- * ARP on active-backup slaves with arp_validate enabled.
>>+ * duplicates except for alb non-mcast/bcast.
>>   */
>>  static bool bond_should_deliver_exact_match(struct sk_buff *skb,
>>  					    struct slave *slave,
>>  					    struct bonding *bond)
>>  {
>>  	if (bond_is_slave_inactive(slave)) {
>>-		if (slave_do_arp_validate(bond, slave)&&
>>-		    skb->protocol == __cpu_to_be16(ETH_P_ARP))
>>-			return false;
>>-
>>  		if (bond->params.mode == BOND_MODE_ALB&&
>>  		skb->pkt_type != PACKET_BROADCAST&&
>>  		skb->pkt_type != PACKET_MULTICAST)
>>-				return false;
>>-
>>-		if (bond->params.mode == BOND_MODE_8023AD&&
>>-		    skb->protocol == __cpu_to_be16(ETH_P_SLOW))
>>  			return false;
>>-
>>  		return true;
>>  	}
>>  	return false;
>>@@ -1513,6 +1503,15 @@ static struct sk_buff *bond_handle_frame(struct sk_buff *skb)
>>  	if (bond->params.arp_interval)
>>  		slave->dev->last_rx = jiffies;
>>
>>+	if (bond->recv_probe) {
>>+		struct sk_buff *nskb = skb_clone(skb, GFP_ATOMIC);
>>+
>>+		if (likely(nskb)) {
>>+			bond->recv_probe(nskb, bond, slave);
>>+			dev_kfree_skb(nskb);
>>+		}
>>+	}
>>+
>>  	if (bond_should_deliver_exact_match(skb, slave, bond)) {
>>  		skb->deliver_no_wcard = 1;
>>  		return skb;
>>@@ -2813,48 +2812,26 @@ static void bond_validate_arp(struct bonding *bond, struct slave *slave, __be32
>>  	}
>>  }
>>
>>-static int bond_arp_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *pt, struct net_device *orig_dev)
>>+static void bond_arp_rcv(struct sk_buff *skb, struct bonding *bond,
>>+			 struct slave *slave)
>>  {
>>  	struct arphdr *arp;
>>-	struct slave *slave;
>>-	struct bonding *bond;
>>  	unsigned char *arp_ptr;
>>  	__be32 sip, tip;
>>
>>-	if (dev->priv_flags&  IFF_802_1Q_VLAN) {
>>-		/*
>>-		 * When using VLANS and bonding, dev and oriv_dev may be
>>-		 * incorrect if the physical interface supports VLAN
>>-		 * acceleration.  With this change ARP validation now
>>-		 * works for hosts only reachable on the VLAN interface.
>>-		 */
>>-		dev = vlan_dev_real_dev(dev);
>>-		orig_dev = dev_get_by_index_rcu(dev_net(skb->dev),skb->skb_iif);
>>-	}
>>-
>>-	if (!(dev->priv_flags&  IFF_BONDING) || !(dev->flags&  IFF_MASTER))
>>-		goto out;
>>+	if (skb->protocol != __cpu_to_be16(ETH_P_ARP))
>>+		return;
>
>What happens if the skb that hold the ARP request/reply is vlan
>tagged? The ARP ptype_base handler for bonding used to be called
>after vlan processing.
>
>>-	bond = netdev_priv(dev);
>>  	read_lock(&bond->lock);
>>
>>-	pr_debug("bond_arp_rcv: bond %s skb->dev %s orig_dev %s\n",
>>-		 bond->dev->name, skb->dev ? skb->dev->name : "NULL",
>>-		 orig_dev ? orig_dev->name : "NULL");
>>+	pr_debug("bond_arp_rcv: bond %s skb->dev %s\n",
>>+		 bond->dev->name, skb->dev->name);
>>
>>-	slave = bond_get_slave_by_dev(bond, orig_dev);
>>-	if (!slave || !slave_do_arp_validate(bond, slave))
>>-		goto out_unlock;
>>-
>>-	skb = skb_share_check(skb, GFP_ATOMIC);
>>-	if (!skb)
>>-		goto out_unlock;
>>-
>>-	if (!pskb_may_pull(skb, arp_hdr_len(dev)))
>>+	if (!pskb_may_pull(skb, arp_hdr_len(bond->dev)))
>>  		goto out_unlock;
>>
>>  	arp = arp_hdr(skb);
>>-	if (arp->ar_hln != dev->addr_len ||
>>+	if (arp->ar_hln != bond->dev->addr_len ||
>>  	    skb->pkt_type == PACKET_OTHERHOST ||
>>  	    skb->pkt_type == PACKET_LOOPBACK ||
>>  	    arp->ar_hrd != htons(ARPHRD_ETHER) ||
>>@@ -2863,9 +2840,9 @@ static int bond_arp_rcv(struct sk_buff *skb, struct net_device *dev, struct pack
>>  		goto out_unlock;
>>
>>  	arp_ptr = (unsigned char *)(arp + 1);
>>-	arp_ptr += dev->addr_len;
>>+	arp_ptr += bond->dev->addr_len;
>>  	memcpy(&sip, arp_ptr, 4);
>>-	arp_ptr += 4 + dev->addr_len;
>>+	arp_ptr += 4 + bond->dev->addr_len;
>>  	memcpy(&tip, arp_ptr, 4);
>>
>>  	pr_debug("bond_arp_rcv: %s %s/%d av %d sv %d sip %pI4 tip %pI4\n",
>>@@ -2888,9 +2865,6 @@ static int bond_arp_rcv(struct sk_buff *skb, struct net_device *dev, struct pack
>>
>>  out_unlock:
>>  	read_unlock(&bond->lock);
>>-out:
>>-	dev_kfree_skb(skb);
>>-	return NET_RX_SUCCESS;
>>  }
>>
>>  /*
>>@@ -3782,48 +3756,6 @@ static struct notifier_block bond_inetaddr_notifier = {
>>  	.notifier_call = bond_inetaddr_event,
>>  };
>>
>>-/*-------------------------- Packet type handling ---------------------------*/
>>-
>>-/* register to receive lacpdus on a bond */
>>-static void bond_register_lacpdu(struct bonding *bond)
>>-{
>>-	struct packet_type *pk_type =&(BOND_AD_INFO(bond).ad_pkt_type);
>>-
>>-	/* initialize packet type */
>>-	pk_type->type = PKT_TYPE_LACPDU;
>>-	pk_type->dev = bond->dev;
>>-	pk_type->func = bond_3ad_lacpdu_recv;
>>-
>>-	dev_add_pack(pk_type);
>>-}
>>-
>>-/* unregister to receive lacpdus on a bond */
>>-static void bond_unregister_lacpdu(struct bonding *bond)
>>-{
>>-	dev_remove_pack(&(BOND_AD_INFO(bond).ad_pkt_type));
>>-}
>>-
>>-void bond_register_arp(struct bonding *bond)
>>-{
>>-	struct packet_type *pt =&bond->arp_mon_pt;
>>-
>>-	if (pt->type)
>>-		return;
>>-
>>-	pt->type = htons(ETH_P_ARP);
>>-	pt->dev = bond->dev;
>>-	pt->func = bond_arp_rcv;
>>-	dev_add_pack(pt);
>>-}
>>-
>>-void bond_unregister_arp(struct bonding *bond)
>>-{
>>-	struct packet_type *pt =&bond->arp_mon_pt;
>>-
>>-	dev_remove_pack(pt);
>>-	pt->type = 0;
>>-}
>>-
>>  /*---------------------------- Hashing Policies -----------------------------*/
>>
>>  /*
>>@@ -3917,14 +3849,14 @@ static int bond_open(struct net_device *bond_dev)
>>
>>  		queue_delayed_work(bond->wq,&bond->arp_work, 0);
>>  		if (bond->params.arp_validate)
>>-			bond_register_arp(bond);
>>+			bond->recv_probe = bond_arp_rcv;
>>  	}
>>
>>  	if (bond->params.mode == BOND_MODE_8023AD) {
>>  		INIT_DELAYED_WORK(&bond->ad_work, bond_3ad_state_machine_handler);
>>  		queue_delayed_work(bond->wq,&bond->ad_work, 0);
>>  		/* register to receive LACPDUs */
>>-		bond_register_lacpdu(bond);
>>+		bond->recv_probe = bond_3ad_lacpdu_recv;
>>  		bond_3ad_initiate_agg_selection(bond, 1);
>>  	}
>>
>>@@ -3935,14 +3867,6 @@ static int bond_close(struct net_device *bond_dev)
>>  {
>>  	struct bonding *bond = netdev_priv(bond_dev);
>>
>>-	if (bond->params.mode == BOND_MODE_8023AD) {
>>-		/* Unregister the receive of LACPDUs */
>>-		bond_unregister_lacpdu(bond);
>>-	}
>>-
>>-	if (bond->params.arp_validate)
>>-		bond_unregister_arp(bond);
>>-
>>  	write_lock_bh(&bond->lock);
>>
>>  	bond->send_grat_arp = 0;
>>@@ -3982,6 +3906,7 @@ static int bond_close(struct net_device *bond_dev)
>>  		 */
>>  		bond_alb_deinitialize(bond);
>>  	}
>>+	bond->recv_probe = NULL;
>>
>>  	return 0;
>>  }
>>diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
>>index 5161183..6804efe 100644
>>--- a/drivers/net/bonding/bond_sysfs.c
>>+++ b/drivers/net/bonding/bond_sysfs.c
>>@@ -419,11 +419,6 @@ static ssize_t bonding_store_arp_validate(struct device *d,
>>  		bond->dev->name, arp_validate_tbl[new_value].modename,
>>  		new_value);
>>
>>-	if (!bond->params.arp_validate&&  new_value)
>>-		bond_register_arp(bond);
>>-	else if (bond->params.arp_validate&&  !new_value)
>>-		bond_unregister_arp(bond);
>>-
>>  	bond->params.arp_validate = new_value;
>>
>>  	return count;
>>@@ -998,7 +993,6 @@ static ssize_t bonding_store_miimon(struct device *d,
>>  				bond->dev->name);
>>  			bond->params.arp_interval = 0;
>>  			if (bond->params.arp_validate) {
>>-				bond_unregister_arp(bond);
>>  				bond->params.arp_validate =
>>  					BOND_ARP_VALIDATE_NONE;
>>  			}
>>diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
>>index 8f78166..75a1308 100644
>>--- a/drivers/net/bonding/bonding.h
>>+++ b/drivers/net/bonding/bonding.h
>>@@ -228,6 +228,8 @@ struct bonding {
>>  	struct   slave *primary_slave;
>>  	bool     force_primary;
>>  	s32      slave_cnt; /* never change this value outside the attach/detach wrappers */
>>+	void     (*recv_probe)(struct sk_buff *, struct bonding *,
>>+			       struct slave *);
>>  	rwlock_t lock;
>>  	rwlock_t curr_slave_lock;
>>  	s8       kill_timers;
>>@@ -406,8 +408,6 @@ void bond_set_mode_ops(struct bonding *bond, int mode);
>>  int bond_parse_parm(const char *mode_arg, const struct bond_parm_tbl *tbl);
>>  void bond_select_active_slave(struct bonding *bond);
>>  void bond_change_active_slave(struct bonding *bond, struct slave *new_active);
>>-void bond_register_arp(struct bonding *);
>>-void bond_unregister_arp(struct bonding *);
>>  void bond_create_debugfs(void);
>>  void bond_destroy_debugfs(void);
>>  void bond_debug_register(struct bonding *bond);
>

^ permalink raw reply

* Re: [patch net-next-2.6 3/8] net: get rid of multiple bond-related netdevice->priv_flags
From: Nicolas de Pesloüan @ 2011-03-05 14:46 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: Jiri Pirko, netdev, davem, shemminger, kaber, fubar, eric.dumazet,
	andy
In-Reply-To: <1299335862.4277.99.camel@localhost>

Le 05/03/2011 15:37, Ben Hutchings a écrit :
> On Sat, 2011-03-05 at 15:14 +0100, Nicolas de Pesloüan wrote:
>> Le 05/03/2011 11:29, Jiri Pirko a écrit :
> [...]
>>> --- a/include/linux/if.h
>>> +++ b/include/linux/if.h
>>> @@ -60,21 +60,17 @@
>>>    #define IFF_802_1Q_VLAN 0x1             /* 802.1Q VLAN device.          */
>>>    #define IFF_EBRIDGE	0x2		/* Ethernet bridging device.	*/
>>>    #define IFF_SLAVE_INACTIVE	0x4	/* bonding slave not the curr. active */
>>> -#define IFF_MASTER_8023AD	0x8	/* bonding master, 802.3ad. 	*/
>>> -#define IFF_MASTER_ALB	0x10		/* bonding master, balance-alb.	*/
>>> -#define IFF_BONDING	0x20		/* bonding master or slave	*/
>>> -#define IFF_SLAVE_NEEDARP 0x40		/* need ARPs for validation	*/
>>> -#define IFF_ISATAP	0x80		/* ISATAP interface (RFC4214)	*/
>>> -#define IFF_MASTER_ARPMON 0x100		/* bonding master, ARP mon in use */
>>> -#define IFF_WAN_HDLC	0x200		/* WAN HDLC device		*/
>>> -#define IFF_XMIT_DST_RELEASE 0x400	/* dev_hard_start_xmit() is allowed to
>>> +#define IFF_BONDING	0x8		/* bonding master or slave	*/
>>> +#define IFF_ISATAP	0x10		/* ISATAP interface (RFC4214)	*/
>>> +#define IFF_WAN_HDLC	0x20		/* WAN HDLC device		*/
>>> +#define IFF_XMIT_DST_RELEASE 0x40	/* dev_hard_start_xmit() is allowed to
>>>    					 * release skb->dst
>>>    					 */
>>
>> Why did you changed those values? Aren't those exposed to userland? Just removing the unused one
>> sounds good to me.
> [...]
>
> Look just one line further up:
>
> /* Private (from user) interface flags (netdevice->priv_flags). */
>
> Maybe they should have a prefix IFPF_ instead of IFF_ though.
>
> Ben.

Yes, I noticed it while reviewing the next one. Sorry for the noise.

So,

Reviewed-by: Nicolas de Pesloüan <nicolas.2p.debian@free.fr>



^ permalink raw reply

* Re: [patch net-next-2.6 6/8] bonding: move processing of recv handlers into handle_frame()
From: Nicolas de Pesloüan @ 2011-03-05 14:50 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: netdev, davem, shemminger, kaber, fubar, eric.dumazet, andy
In-Reply-To: <20110305144314.GC8573@psychotron.redhat.com>

Le 05/03/2011 15:43, Jiri Pirko a écrit :
> Sat, Mar 05, 2011 at 03:33:30PM CET, nicolas.2p.debian@gmail.com wrote:
>> Le 05/03/2011 11:29, Jiri Pirko a écrit :
>>> Since now when bonding uses rx_handler, all traffic going into bond
>>> device goes thru bond_handle_frame. So there's no need to go back into
>>> bonding code later via ptype handlers. This patch converts
>>> original ptype handlers into "bonding receive probes". These functions
>>> are called from bond_handle_frame and they are registered per-mode.
>>
>> Does this still support having the arp_ip_target on a vlan?
>>
>> (eth0 ->  bond0 ->  bond0.100, with arp_ip_target only reachable through bond0.100).
>
> This case is still covered with vlan_on_bond_hook
> eth0->
> 	bond_handle_frame
> bond0->
> 	vlan_hwaccel_do_receive
> bond0.5->
> 	vlan_on_bond_hook ->  reinject into bond0
> 				->  bond_handle_frame (here it is processed)

Sound good to me.

Reviewed-by: Nicolas de Pesloüan <nicolas.2p.debian@free.fr>

^ permalink raw reply

* Re: [patch net-next-2.6 7/8] net: introduce rx_handler results and logic around that
From: Nicolas de Pesloüan @ 2011-03-05 14:52 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: Ben Hutchings, netdev, davem, shemminger, kaber, fubar,
	eric.dumazet, andy
In-Reply-To: <1299329330.4277.97.camel@localhost>

Le 05/03/2011 13:48, Ben Hutchings a écrit :
> On Sat, 2011-03-05 at 11:29 +0100, Jiri Pirko wrote:
>> This patch allows rx_handlers to better signalize what to do next to
>> it's caller. That makes skb->deliver_no_wcard no longer needed.
> [...]
>> --- a/include/linux/netdevice.h
>> +++ b/include/linux/netdevice.h
>> @@ -390,7 +390,14 @@ enum gro_result {
>>   };
>>   typedef enum gro_result gro_result_t;
>>
>> -typedef struct sk_buff *rx_handler_func_t(struct sk_buff *skb);
>> +enum rx_handler_result {
>> +	RX_HANDLER_CONSUMED,
>> +	RX_HANDLER_ANOTHER,
>> +	RX_HANDLER_EXACT,
>> +	RX_HANDLER_PASS,
>> +};
> [...]
>
> This should have a comment (preferably kernel-doc) clearly specifying
> the meaning of each code, as the differences between ANOTHER/EXACT/PASS
> are fairly subtle.
>
> Ben,

Except from the lack of proper documentation, this patch looks very good.

	Nicolas.

^ permalink raw reply

* Re: [patch net-next-2.6 7/8] net: introduce rx_handler results and logic around that
From: Jiri Pirko @ 2011-03-05 14:54 UTC (permalink / raw)
  To: Nicolas de Pesloüan
  Cc: Ben Hutchings, netdev, davem, shemminger, kaber, fubar,
	eric.dumazet, andy
In-Reply-To: <4D724E23.7020904@gmail.com>

Sat, Mar 05, 2011 at 03:52:19PM CET, nicolas.2p.debian@gmail.com wrote:
>Le 05/03/2011 13:48, Ben Hutchings a écrit :
>>On Sat, 2011-03-05 at 11:29 +0100, Jiri Pirko wrote:
>>>This patch allows rx_handlers to better signalize what to do next to
>>>it's caller. That makes skb->deliver_no_wcard no longer needed.
>>[...]
>>>--- a/include/linux/netdevice.h
>>>+++ b/include/linux/netdevice.h
>>>@@ -390,7 +390,14 @@ enum gro_result {
>>>  };
>>>  typedef enum gro_result gro_result_t;
>>>
>>>-typedef struct sk_buff *rx_handler_func_t(struct sk_buff *skb);
>>>+enum rx_handler_result {
>>>+	RX_HANDLER_CONSUMED,
>>>+	RX_HANDLER_ANOTHER,
>>>+	RX_HANDLER_EXACT,
>>>+	RX_HANDLER_PASS,
>>>+};
>>[...]
>>
>>This should have a comment (preferably kernel-doc) clearly specifying
>>the meaning of each code, as the differences between ANOTHER/EXACT/PASS
>>are fairly subtle.
>>
>>Ben,
>
>Except from the lack of proper documentation, this patch looks very good.

Okay guys, I'll write something about that and send it in another patch.

>
>	Nicolas.

^ permalink raw reply

* Re: [patch net-next-2.6 8/8] net: get rid of orig_dev parameter of packet handlers
From: Nicolas de Pesloüan @ 2011-03-05 15:05 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: netdev, davem, shemminger, kaber, fubar, eric.dumazet, andy
In-Reply-To: <1299320969-7951-9-git-send-email-jpirko@redhat.com>

Le 05/03/2011 11:29, Jiri Pirko a écrit :
> Since orig_dev was used in af_packet (where it was replaced) and by
> bonding ptype handlers (which are no longer with us) only, orig_dev
> parameter is no longer needed ->  toss it out
>
> Signed-off-by: Jiri Pirko<jpirko@redhat.com>

[snip]
> -	new->deliver_no_wcard	= old->deliver_no_wcard;

deliver_no_wcard was removed from sk_buff in your #7 patch. Shouldn't the above line be part of the 
#7 patch?

Except from this point,

Reviewed-by: Nicolas de Pesloüan <nicolas.2p.debian@free.fr>

	Nicolas.

^ permalink raw reply

* Re: [patch net-next-2.6 7/8] net: introduce rx_handler results and logic around that
From: Nicolas de Pesloüan @ 2011-03-05 15:06 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: Ben Hutchings, netdev, davem, shemminger, kaber, fubar,
	eric.dumazet, andy
In-Reply-To: <20110305145408.GD8573@psychotron.redhat.com>

Le 05/03/2011 15:54, Jiri Pirko a écrit :
> Sat, Mar 05, 2011 at 03:52:19PM CET, nicolas.2p.debian@gmail.com wrote:
>> Le 05/03/2011 13:48, Ben Hutchings a écrit :
>>> On Sat, 2011-03-05 at 11:29 +0100, Jiri Pirko wrote:
>>>> This patch allows rx_handlers to better signalize what to do next to
>>>> it's caller. That makes skb->deliver_no_wcard no longer needed.
>>> [...]
>>>> --- a/include/linux/netdevice.h
>>>> +++ b/include/linux/netdevice.h
>>>> @@ -390,7 +390,14 @@ enum gro_result {
>>>>   };
>>>>   typedef enum gro_result gro_result_t;
>>>>
>>>> -typedef struct sk_buff *rx_handler_func_t(struct sk_buff *skb);
>>>> +enum rx_handler_result {
>>>> +	RX_HANDLER_CONSUMED,
>>>> +	RX_HANDLER_ANOTHER,
>>>> +	RX_HANDLER_EXACT,
>>>> +	RX_HANDLER_PASS,
>>>> +};
>>> [...]
>>>
>>> This should have a comment (preferably kernel-doc) clearly specifying
>>> the meaning of each code, as the differences between ANOTHER/EXACT/PASS
>>> are fairly subtle.
>>>
>>> Ben,
>>
>> Except from the lack of proper documentation, this patch looks very good.
>
> Okay guys, I'll write something about that and send it in another patch.

Agreed that it can be in a follow-up patch.

Reviewed-by: Nicolas de Pesloüan <nicolas.2p.debian@free.fr>

^ permalink raw reply

* [patch net-next-2.6] net: comment rx_handler results
From: Jiri Pirko @ 2011-03-05 15:13 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: netdev, davem, shemminger, kaber, fubar, eric.dumazet,
	nicolas.2p.debian, andy
In-Reply-To: <1299329330.4277.97.camel@localhost>


Signed-off-by: Jiri Pirko <jpirko@redhat.com>
---
 include/linux/netdevice.h |   12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 48a9638..26e03f9 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -391,10 +391,14 @@ enum gro_result {
 typedef enum gro_result gro_result_t;
 
 enum rx_handler_result {
-	RX_HANDLER_CONSUMED,
-	RX_HANDLER_ANOTHER,
-	RX_HANDLER_EXACT,
-	RX_HANDLER_PASS,
+	RX_HANDLER_CONSUMED,	/* skb was consumed by rx_handler,
+				   do not process it further. */
+	RX_HANDLER_ANOTHER,	/* Do another round in receive path.
+				   This is indicated in case skb->dev
+				   was changed by rx_handler */
+	RX_HANDLER_EXACT,	/* Force exact delivery, no wildcard */
+	RX_HANDLER_PASS,	/* Do nothing, pass the skb as if
+				   no rx_handler was called */
 };
 typedef enum rx_handler_result rx_handler_result_t;
 typedef rx_handler_result_t rx_handler_func_t(struct sk_buff **pskb);
-- 
1.7.4


^ permalink raw reply related

* Re: [patch net-next-2.6 8/8] net: get rid of orig_dev parameter of packet handlers
From: Jiri Pirko @ 2011-03-05 15:15 UTC (permalink / raw)
  To: Nicolas de Pesloüan
  Cc: netdev, davem, shemminger, kaber, fubar, eric.dumazet, andy
In-Reply-To: <4D72512C.2060802@gmail.com>

Sat, Mar 05, 2011 at 04:05:16PM CET, nicolas.2p.debian@gmail.com wrote:
>Le 05/03/2011 11:29, Jiri Pirko a écrit :
>>Since orig_dev was used in af_packet (where it was replaced) and by
>>bonding ptype handlers (which are no longer with us) only, orig_dev
>>parameter is no longer needed ->  toss it out
>>
>>Signed-off-by: Jiri Pirko<jpirko@redhat.com>
>
>[snip]
>>-	new->deliver_no_wcard	= old->deliver_no_wcard;
>
>deliver_no_wcard was removed from sk_buff in your #7 patch. Shouldn't
>the above line be part of the #7 patch?

Indeed, I wonder how if got here. Will repost to correct this.
>
>Except from this point,
>
>Reviewed-by: Nicolas de Pesloüan <nicolas.2p.debian@free.fr>
>
>	Nicolas.

^ permalink raw reply

* Re: [patch net-next-2.6 4/8] bonding: wrap slave state work
From: Nicolas de Pesloüan @ 2011-03-05 15:21 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: netdev, davem, shemminger, kaber, fubar, eric.dumazet, andy
In-Reply-To: <1299320969-7951-5-git-send-email-jpirko@redhat.com>

Le 05/03/2011 11:29, Jiri Pirko a écrit :
> transfers slave->state into slave->backup (that it's going to transfer
> into bitfield. Introduce wrapper inlines to do the work with it.
>
> Signed-off-by: Jiri Pirko<jpirko@redhat.com>
> ---
>   drivers/net/bonding/bond_3ad.c   |    2 +-
>   drivers/net/bonding/bond_main.c  |   36 ++++++++++++++++++------------------
>   drivers/net/bonding/bond_sysfs.c |    2 +-
>   drivers/net/bonding/bonding.h    |   31 ++++++++++++++++++++++++++-----
>   4 files changed, 46 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
> index 1024ae1..047af0b 100644
> --- a/drivers/net/bonding/bond_3ad.c
> +++ b/drivers/net/bonding/bond_3ad.c
> @@ -246,7 +246,7 @@ static inline void __enable_port(struct port *port)
>    */
>   static inline int __port_is_enabled(struct port *port)
>   {
> -	return port->slave->state == BOND_STATE_ACTIVE;
> +	return bond_is_active_slave(port->slave);
>   }
>
>   /**
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 7923184..62020a7 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -1872,7 +1872,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
>   		break;
>   	case BOND_MODE_TLB:
>   	case BOND_MODE_ALB:
> -		new_slave->state = BOND_STATE_ACTIVE;
> +		bond_set_active_slave(new_slave);
>   		bond_set_slave_inactive_flags(new_slave);
>   		bond_select_active_slave(bond);
>   		break;
> @@ -1880,7 +1880,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
>   		pr_debug("This slave is always active in trunk mode\n");
>
>   		/* always active in trunk mode */
> -		new_slave->state = BOND_STATE_ACTIVE;
> +		bond_set_active_slave(new_slave);
>
>   		/* In trunking mode there is little meaning to curr_active_slave
>   		 * anyway (it holds no special properties of the bond device),
> @@ -1918,7 +1918,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
>
>   	pr_info("%s: enslaving %s as a%s interface with a%s link.\n",
>   		bond_dev->name, slave_dev->name,
> -		new_slave->state == BOND_STATE_ACTIVE ? "n active" : " backup",
> +		bond_is_active_slave(new_slave) ? "n active" : " backup",
 >   		new_slave->link != BOND_LINK_DOWN ? "n up" : " down");

I would prefer the following, for the benefit of non native English readers:

    	pr_info("%s: enslaving %s as %s interface with %s link.\n",
  		bond_is_active_slave(new_slave) ? "an active" : "a backup",
    		new_slave->link != BOND_LINK_DOWN ? "an up" : "a down");

It can be in a follow-up patch.

>
>   	/* enslave is successful */
> @@ -2016,7 +2016,7 @@ int bond_release(struct net_device *bond_dev, struct net_device *slave_dev)
>
>   	pr_info("%s: releasing %s interface %s\n",
>   		bond_dev->name,
> -		(slave->state == BOND_STATE_ACTIVE) ? "active" : "backup",
> +		bond_is_active_slave(slave) ? "active" : "backup",
>   		slave_dev->name);
>
>   	oldcurrent = bond->curr_active_slave;
> @@ -2357,7 +2357,7 @@ static int bond_slave_info_query(struct net_device *bond_dev, struct ifslave *in
>   			res = 0;
>   			strcpy(info->slave_name, slave->dev->name);
>   			info->link = slave->link;
> -			info->state = slave->state;
> +			info->state = bond_slave_state(slave);
>   			info->link_failure_count = slave->link_failure_count;
>   			break;
>   		}
> @@ -2396,7 +2396,7 @@ static int bond_miimon_inspect(struct bonding *bond)
>   					bond->dev->name,
>   					(bond->params.mode ==
>   					 BOND_MODE_ACTIVEBACKUP) ?
> -					((slave->state == BOND_STATE_ACTIVE) ?
> +					(bond_is_active_slave(slave) ?
>   					 "active " : "backup ") : "",
>   					slave->dev->name,
>   					bond->params.downdelay * bond->params.miimon);
> @@ -2487,13 +2487,13 @@ static void bond_miimon_commit(struct bonding *bond)
>
>   			if (bond->params.mode == BOND_MODE_8023AD) {
>   				/* prevent it from being the active one */
> -				slave->state = BOND_STATE_BACKUP;
> +				bond_set_backup_slave(slave);
>   			} else if (bond->params.mode != BOND_MODE_ACTIVEBACKUP) {
>   				/* make it immediately active */
> -				slave->state = BOND_STATE_ACTIVE;
> +				bond_set_active_slave(slave);
>   			} else if (slave != bond->primary_slave) {
>   				/* prevent it from being the active one */
> -				slave->state = BOND_STATE_BACKUP;
> +				bond_set_backup_slave(slave);
>   			}
>
>   			bond_update_speed_duplex(slave);
> @@ -2871,7 +2871,7 @@ static int bond_arp_rcv(struct sk_buff *skb, struct net_device *dev, struct pack
>   	memcpy(&tip, arp_ptr, 4);
>
>   	pr_debug("bond_arp_rcv: %s %s/%d av %d sv %d sip %pI4 tip %pI4\n",
> -		 bond->dev->name, slave->dev->name, slave->state,
> +		 bond->dev->name, slave->dev->name, bond_slave_state(slave),
>   		 bond->params.arp_validate, slave_do_arp_validate(bond, slave),
>   		&sip,&tip);
>
> @@ -2883,7 +2883,7 @@ static int bond_arp_rcv(struct sk_buff *skb, struct net_device *dev, struct pack
>   	 * the active, through one switch, the router, then the other
>   	 * switch before reaching the backup.
>   	 */
> -	if (slave->state == BOND_STATE_ACTIVE)
> +	if (bond_is_active_slave(slave))
>   		bond_validate_arp(bond, slave, sip, tip);
>   	else
>   		bond_validate_arp(bond, slave, tip, sip);
> @@ -2945,7 +2945,7 @@ void bond_loadbalance_arp_mon(struct work_struct *work)
>   				slave->dev->last_rx + delta_in_ticks)) {
>
>   				slave->link  = BOND_LINK_UP;
> -				slave->state = BOND_STATE_ACTIVE;
> +				bond_set_active_slave(slave);
>
>   				/* primary_slave has no meaning in round-robin
>   				 * mode. the window of a slave being up and
> @@ -2978,7 +2978,7 @@ void bond_loadbalance_arp_mon(struct work_struct *work)
>   				slave->dev->last_rx + 2 * delta_in_ticks)) {
>
>   				slave->link  = BOND_LINK_DOWN;
> -				slave->state = BOND_STATE_BACKUP;
> +				bond_set_backup_slave(slave);
>
>   				if (slave->link_failure_count<  UINT_MAX)
>   					slave->link_failure_count++;
> @@ -3072,7 +3072,7 @@ static int bond_ab_arp_inspect(struct bonding *bond, int delta_in_ticks)
>   		 * gives each slave a chance to tx/rx traffic
>   		 * before being taken out
>   		 */
> -		if (slave->state == BOND_STATE_BACKUP&&
> +		if (!bond_is_active_slave(slave)&&
>   		!bond->current_arp_slave&&
>   		!time_in_range(jiffies,
>   			slave_last_rx(bond, slave) - delta_in_ticks,
> @@ -3089,7 +3089,7 @@ static int bond_ab_arp_inspect(struct bonding *bond, int delta_in_ticks)
>   		 *    the bond has an IP address)
>   		 */
>   		trans_start = dev_trans_start(slave->dev);
> -		if ((slave->state == BOND_STATE_ACTIVE)&&
> +		if (bond_is_active_slave(slave)&&
>   		(!time_in_range(jiffies,
>   			trans_start - delta_in_ticks,
>   			trans_start + 2 * delta_in_ticks) ||
> @@ -4446,7 +4446,7 @@ static int bond_xmit_roundrobin(struct sk_buff *skb, struct net_device *bond_dev
>   	bond_for_each_slave_from(bond, slave, i, start_at) {
>   		if (IS_UP(slave->dev)&&
>   		(slave->link == BOND_LINK_UP)&&
> -		    (slave->state == BOND_STATE_ACTIVE)) {
> +		    bond_is_active_slave(slave)) {
>   			res = bond_dev_queue_xmit(bond, skb, slave->dev);
>   			break;
>   		}
> @@ -4523,7 +4523,7 @@ static int bond_xmit_xor(struct sk_buff *skb, struct net_device *bond_dev)
>   	bond_for_each_slave_from(bond, slave, i, start_at) {
>   		if (IS_UP(slave->dev)&&
>   		(slave->link == BOND_LINK_UP)&&
> -		    (slave->state == BOND_STATE_ACTIVE)) {
> +		    bond_is_active_slave(slave)) {
>   			res = bond_dev_queue_xmit(bond, skb, slave->dev);
>   			break;
>   		}
> @@ -4564,7 +4564,7 @@ static int bond_xmit_broadcast(struct sk_buff *skb, struct net_device *bond_dev)
>   	bond_for_each_slave_from(bond, slave, i, start_at) {
>   		if (IS_UP(slave->dev)&&
>   		(slave->link == BOND_LINK_UP)&&
> -		    (slave->state == BOND_STATE_ACTIVE)) {
> +		    bond_is_active_slave(slave)) {
>   			if (tx_dev) {
>   				struct sk_buff *skb2 = skb_clone(skb, GFP_ATOMIC);
>   				if (!skb2) {
> diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
> index 05e0ae5..344d23f 100644
> --- a/drivers/net/bonding/bond_sysfs.c
> +++ b/drivers/net/bonding/bond_sysfs.c
> @@ -1579,7 +1579,7 @@ static ssize_t bonding_store_slaves_active(struct device *d,
>   	}
>
>   	bond_for_each_slave(bond, slave, i) {
> -		if (slave->state == BOND_STATE_BACKUP) {
> +		if (!bond_is_active_slave(slave)) {
>   			if (new_value)
>   				slave->dev->priv_flags&= ~IFF_SLAVE_INACTIVE;
>   			else
> diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
> index ddee62f..8a3718b 100644
> --- a/drivers/net/bonding/bonding.h
> +++ b/drivers/net/bonding/bonding.h
> @@ -53,7 +53,7 @@
>   		    (((slave)->dev->flags&  IFF_UP)&&  \
>   		     netif_running((slave)->dev)&&  \
>   		     ((slave)->link == BOND_LINK_UP)&&  \
> -		     ((slave)->state == BOND_STATE_ACTIVE))
> +		     bond_is_active_slave(slave))
>
>
>   #define USES_PRIMARY(mode)				\
> @@ -190,7 +190,8 @@ struct slave {
>   	unsigned long last_arp_rx;
>   	s8     link;    /* one of BOND_LINK_XXXX */
>   	s8     new_link;
> -	s8     state;   /* one of BOND_STATE_XXXX */
> +	u8     backup;	/* indicates backup slave. Value corresponds with
> +			   BOND_STATE_ACTIVE and BOND_STATE_BACKUP */
>   	u32    original_mtu;
>   	u32    link_failure_count;
>   	u8     perm_hwaddr[ETH_ALEN];
> @@ -302,6 +303,26 @@ static inline bool bond_is_lb(const struct bonding *bond)
>   		bond->params.mode == BOND_MODE_ALB);
>   }
>
> +static inline void bond_set_active_slave(struct slave *slave)
> +{
> +	slave->backup = 0;

In the comment above, you said that the possible value for backup corresponds with BOND_STATE_ACTIVE 
and BOND_STATE_BACKUP.

So, should be:

slave->backup = BOND_STATE_ACTIVE;

> +}
> +
> +static inline void bond_set_backup_slave(struct slave *slave)
> +{
> +	slave->backup = 1;

slave->backup = BOND_STATE_BACKUP;

> +}
> +
> +static inline int bond_slave_state(struct slave *slave)
> +{
> +	return slave->backup;
> +}
> +
> +static inline bool bond_is_active_slave(struct slave *slave)
> +{
> +	return !bond_slave_state(slave);
> +}
> +
>   #define BOND_PRI_RESELECT_ALWAYS	0
>   #define BOND_PRI_RESELECT_BETTER	1
>   #define BOND_PRI_RESELECT_FAILURE	2
> @@ -319,7 +340,7 @@ static inline bool bond_is_lb(const struct bonding *bond)
>   static inline int slave_do_arp_validate(struct bonding *bond,
>   					struct slave *slave)
>   {
> -	return bond->params.arp_validate&  (1<<  slave->state);
> +	return bond->params.arp_validate&  (1<<  bond_slave_state(slave));
>   }
>
>   static inline unsigned long slave_last_rx(struct bonding *bond,
> @@ -351,14 +372,14 @@ static inline void bond_set_slave_inactive_flags(struct slave *slave)
>   {
>   	struct bonding *bond = netdev_priv(slave->dev->master);
>   	if (!bond_is_lb(bond))
> -		slave->state = BOND_STATE_BACKUP;
> +		bond_set_backup_slave(slave);
>   	if (!bond->params.all_slaves_active)
>   		slave->dev->priv_flags |= IFF_SLAVE_INACTIVE;
>   }
>
>   static inline void bond_set_slave_active_flags(struct slave *slave)
>   {
> -	slave->state = BOND_STATE_ACTIVE;
> +	bond_set_active_slave(slave);
>   	slave->dev->priv_flags&= ~IFF_SLAVE_INACTIVE;
>   }
>


^ permalink raw reply

* Re: [patch net-next-2.6] net: comment rx_handler results
From: Nicolas de Pesloüan @ 2011-03-05 15:27 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: Ben Hutchings, netdev, davem, shemminger, kaber, fubar,
	eric.dumazet, andy
In-Reply-To: <20110305151331.GE8573@psychotron.redhat.com>

Le 05/03/2011 16:13, Jiri Pirko a écrit :
>
> Signed-off-by: Jiri Pirko<jpirko@redhat.com>
> ---
>   include/linux/netdevice.h |   12 ++++++++----
>   1 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 48a9638..26e03f9 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -391,10 +391,14 @@ enum gro_result {
>   typedef enum gro_result gro_result_t;
>
>   enum rx_handler_result {
> -	RX_HANDLER_CONSUMED,
> -	RX_HANDLER_ANOTHER,
> -	RX_HANDLER_EXACT,
> -	RX_HANDLER_PASS,
> +	RX_HANDLER_CONSUMED,	/* skb was consumed by rx_handler,
> +				   do not process it further. */
> +	RX_HANDLER_ANOTHER,	/* Do another round in receive path.
> +				   This is indicated in case skb->dev
> +				   was changed by rx_handler */
> +	RX_HANDLER_EXACT,	/* Force exact delivery, no wildcard */
> +	RX_HANDLER_PASS,	/* Do nothing, pass the skb as if
> +				   no rx_handler was called */

Changing skb->dev should always cause another_round.

So, RX_HANDLER_ANOTHER could be removed and replaced with a test for skb->dev change, with 
RX_HANDER_EXACT and RX_HANDLER_PASS.

 > +	RX_HANDLER_CONSUMED,	/* skb was consumed by rx_handler,
 > +				   do not process it further. */
 > +	RX_HANDLER_EXACT,	/* Force exact delivery, no wildcard */
 > +	RX_HANDLER_PASS,	/* Do normal (wildcard and exact) delivery */

And any rx_handler returning RX_HANDLER_EXACT should cause exact delivery, even if possible next 
rx_handlers return RX_HANDLER_PASS.

	Nicolas.

^ permalink raw reply

* [patch net-next-2.6 7/8 v2] net: introduce rx_handler results and logic around that
From: Jiri Pirko @ 2011-03-05 15:31 UTC (permalink / raw)
  To: netdev
  Cc: davem, shemminger, kaber, fubar, eric.dumazet, nicolas.2p.debian,
	andy
In-Reply-To: <1299320969-7951-8-git-send-email-jpirko@redhat.com>

This patch allows rx_handlers to better signalize what to do next to
it's caller. That makes skb->deliver_no_wcard no longer needed.

Signed-off-by: Jiri Pirko <jpirko@redhat.com>
Reviewed-by: Nicolas de Pesloüan <nicolas.2p.debian@free.fr>

v1->v2:
	moved __copy_skb_header hunk from 8/8 here

---
 drivers/net/bonding/bond_main.c |   20 +++++++++++---------
 drivers/net/macvlan.c           |   11 ++++++-----
 include/linux/netdevice.h       |    9 ++++++++-
 include/linux/skbuff.h          |    5 +----
 net/bridge/br_input.c           |   25 +++++++++++++++----------
 net/bridge/br_private.h         |    2 +-
 net/core/dev.c                  |   19 ++++++++++++-------
 net/core/skbuff.c               |    1 -
 8 files changed, 54 insertions(+), 38 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 8ba7faa..ab56190 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1483,20 +1483,23 @@ static bool bond_should_deliver_exact_match(struct sk_buff *skb,
 	return false;
 }
 
-static struct sk_buff *bond_handle_frame(struct sk_buff *skb)
+static rx_handler_result_t bond_handle_frame(struct sk_buff **pskb)
 {
+	struct sk_buff *skb = *pskb;
 	struct slave *slave;
 	struct net_device *bond_dev;
 	struct bonding *bond;
 
-	skb = skb_share_check(skb, GFP_ATOMIC);
-	if (unlikely(!skb))
-		return NULL;
-
 	slave = bond_slave_get_rcu(skb->dev);
 	bond_dev = ACCESS_ONCE(slave->dev->master);
 	if (unlikely(!bond_dev))
-		return skb;
+		return RX_HANDLER_PASS;
+
+	skb = skb_share_check(skb, GFP_ATOMIC);
+	if (unlikely(!skb))
+		return RX_HANDLER_CONSUMED;
+
+	*pskb = skb;
 
 	bond = netdev_priv(bond_dev);
 
@@ -1513,8 +1516,7 @@ static struct sk_buff *bond_handle_frame(struct sk_buff *skb)
 	}
 
 	if (bond_should_deliver_exact_match(skb, slave, bond)) {
-		skb->deliver_no_wcard = 1;
-		return skb;
+		return RX_HANDLER_EXACT;
 	}
 
 	skb->dev = bond_dev;
@@ -1527,7 +1529,7 @@ static struct sk_buff *bond_handle_frame(struct sk_buff *skb)
 		memcpy(dest, bond_dev->dev_addr, ETH_ALEN);
 	}
 
-	return skb;
+	return RX_HANDLER_ANOTHER;
 }
 
 /* enslave device <slave> to bond device <master> */
diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 6ed577b..ead9a8f 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -152,9 +152,10 @@ static void macvlan_broadcast(struct sk_buff *skb,
 }
 
 /* called under rcu_read_lock() from netif_receive_skb */
-static struct sk_buff *macvlan_handle_frame(struct sk_buff *skb)
+static rx_handler_result_t macvlan_handle_frame(struct sk_buff **pskb)
 {
 	struct macvlan_port *port;
+	struct sk_buff *skb = *pskb;
 	const struct ethhdr *eth = eth_hdr(skb);
 	const struct macvlan_dev *vlan;
 	const struct macvlan_dev *src;
@@ -184,7 +185,7 @@ static struct sk_buff *macvlan_handle_frame(struct sk_buff *skb)
 			 */
 			macvlan_broadcast(skb, port, src->dev,
 					  MACVLAN_MODE_VEPA);
-		return skb;
+		return RX_HANDLER_PASS;
 	}
 
 	if (port->passthru)
@@ -192,12 +193,12 @@ static struct sk_buff *macvlan_handle_frame(struct sk_buff *skb)
 	else
 		vlan = macvlan_hash_lookup(port, eth->h_dest);
 	if (vlan == NULL)
-		return skb;
+		return RX_HANDLER_PASS;
 
 	dev = vlan->dev;
 	if (unlikely(!(dev->flags & IFF_UP))) {
 		kfree_skb(skb);
-		return NULL;
+		return RX_HANDLER_CONSUMED;
 	}
 	len = skb->len + ETH_HLEN;
 	skb = skb_share_check(skb, GFP_ATOMIC);
@@ -211,7 +212,7 @@ static struct sk_buff *macvlan_handle_frame(struct sk_buff *skb)
 
 out:
 	macvlan_count_rx(vlan, len, ret == NET_RX_SUCCESS, 0);
-	return NULL;
+	return RX_HANDLER_CONSUMED;
 }
 
 static int macvlan_queue_xmit(struct sk_buff *skb, struct net_device *dev)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 8be4056..ff386a4 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -390,7 +390,14 @@ enum gro_result {
 };
 typedef enum gro_result gro_result_t;
 
-typedef struct sk_buff *rx_handler_func_t(struct sk_buff *skb);
+enum rx_handler_result {
+	RX_HANDLER_CONSUMED,
+	RX_HANDLER_ANOTHER,
+	RX_HANDLER_EXACT,
+	RX_HANDLER_PASS,
+};
+typedef enum rx_handler_result rx_handler_result_t;
+typedef rx_handler_result_t rx_handler_func_t(struct sk_buff **pskb);
 
 extern void __napi_schedule(struct napi_struct *n);
 
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 31f02d0..24cfa62 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -388,10 +388,7 @@ struct sk_buff {
 	kmemcheck_bitfield_begin(flags2);
 	__u16			queue_mapping:16;
 #ifdef CONFIG_IPV6_NDISC_NODETYPE
-	__u8			ndisc_nodetype:2,
-				deliver_no_wcard:1;
-#else
-	__u8			deliver_no_wcard:1;
+	__u8			ndisc_nodetype:2;
 #endif
 	__u8			ooo_okay:1;
 	kmemcheck_bitfield_end(flags2);
diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index 88e4aa9..e216079 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -139,21 +139,22 @@ static inline int is_link_local(const unsigned char *dest)
  * Return NULL if skb is handled
  * note: already called with rcu_read_lock
  */
-struct sk_buff *br_handle_frame(struct sk_buff *skb)
+rx_handler_result_t br_handle_frame(struct sk_buff **pskb)
 {
 	struct net_bridge_port *p;
+	struct sk_buff *skb = *pskb;
 	const unsigned char *dest = eth_hdr(skb)->h_dest;
 	br_should_route_hook_t *rhook;
 
 	if (unlikely(skb->pkt_type == PACKET_LOOPBACK))
-		return skb;
+		return RX_HANDLER_PASS;
 
 	if (!is_valid_ether_addr(eth_hdr(skb)->h_source))
 		goto drop;
 
 	skb = skb_share_check(skb, GFP_ATOMIC);
 	if (!skb)
-		return NULL;
+		return RX_HANDLER_CONSUMED;
 
 	p = br_port_get_rcu(skb->dev);
 
@@ -167,10 +168,12 @@ struct sk_buff *br_handle_frame(struct sk_buff *skb)
 			goto forward;
 
 		if (NF_HOOK(NFPROTO_BRIDGE, NF_BR_LOCAL_IN, skb, skb->dev,
-			    NULL, br_handle_local_finish))
-			return NULL;	/* frame consumed by filter */
-		else
-			return skb;	/* continue processing */
+			    NULL, br_handle_local_finish)) {
+			return RX_HANDLER_CONSUMED; /* consumed by filter */
+		} else {
+			*pskb = skb;
+			return RX_HANDLER_PASS;	/* continue processing */
+		}
 	}
 
 forward:
@@ -178,8 +181,10 @@ forward:
 	case BR_STATE_FORWARDING:
 		rhook = rcu_dereference(br_should_route_hook);
 		if (rhook) {
-			if ((*rhook)(skb))
-				return skb;
+			if ((*rhook)(skb)) {
+				*pskb = skb;
+				return RX_HANDLER_PASS;
+			}
 			dest = eth_hdr(skb)->h_dest;
 		}
 		/* fall through */
@@ -194,5 +199,5 @@ forward:
 drop:
 		kfree_skb(skb);
 	}
-	return NULL;
+	return RX_HANDLER_CONSUMED;
 }
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index f7afc36..19e2f46 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -379,7 +379,7 @@ extern void br_features_recompute(struct net_bridge *br);
 
 /* br_input.c */
 extern int br_handle_frame_finish(struct sk_buff *skb);
-extern struct sk_buff *br_handle_frame(struct sk_buff *skb);
+extern rx_handler_result_t br_handle_frame(struct sk_buff **pskb);
 
 /* br_ioctl.c */
 extern int br_dev_ioctl(struct net_device *dev, struct ifreq *rq, int cmd);
diff --git a/net/core/dev.c b/net/core/dev.c
index 9f66de9..58daddb 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3121,6 +3121,7 @@ static int __netif_receive_skb(struct sk_buff *skb)
 	rx_handler_func_t *rx_handler;
 	struct net_device *orig_dev;
 	struct net_device *null_or_dev;
+	bool deliver_exact = false;
 	int ret = NET_RX_DROP;
 	__be16 type;
 
@@ -3173,18 +3174,22 @@ ncls:
 
 	rx_handler = rcu_dereference(skb->dev->rx_handler);
 	if (rx_handler) {
-		struct net_device *prev_dev;
-
 		if (pt_prev) {
 			ret = deliver_skb(skb, pt_prev, orig_dev);
 			pt_prev = NULL;
 		}
-		prev_dev = skb->dev;
-		skb = rx_handler(skb);
-		if (!skb)
+		switch (rx_handler(&skb)) {
+		case RX_HANDLER_CONSUMED:
 			goto out;
-		if (skb->dev != prev_dev)
+		case RX_HANDLER_ANOTHER:
 			goto another_round;
+		case RX_HANDLER_EXACT:
+			deliver_exact = true;
+		case RX_HANDLER_PASS:
+			break;
+		default:
+			BUG();
+		}
 	}
 
 	if (vlan_tx_tag_present(skb)) {
@@ -3202,7 +3207,7 @@ ncls:
 	vlan_on_bond_hook(skb);
 
 	/* deliver only exact match when indicated */
-	null_or_dev = skb->deliver_no_wcard ? skb->dev : NULL;
+	null_or_dev = deliver_exact ? skb->dev : NULL;
 
 	type = skb->protocol;
 	list_for_each_entry_rcu(ptype,
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 1eb526a..801dd08 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -523,7 +523,6 @@ static void __copy_skb_header(struct sk_buff *new, const struct sk_buff *old)
 	new->ip_summed		= old->ip_summed;
 	skb_copy_queue_mapping(new, old);
 	new->priority		= old->priority;
-	new->deliver_no_wcard	= old->deliver_no_wcard;
 #if defined(CONFIG_IP_VS) || defined(CONFIG_IP_VS_MODULE)
 	new->ipvs_property	= old->ipvs_property;
 #endif
-- 
1.7.4


^ permalink raw reply related

* [patch net-next-2.6 8/8 v2] net: get rid of orig_dev parameter of packet handlers
From: Jiri Pirko @ 2011-03-05 15:32 UTC (permalink / raw)
  To: netdev
  Cc: davem, shemminger, kaber, fubar, eric.dumazet, nicolas.2p.debian,
	andy
In-Reply-To: <1299320969-7951-9-git-send-email-jpirko@redhat.com>

Since orig_dev was used in af_packet (where it was replaced) and by
bonding ptype handlers (which are no longer with us) only, orig_dev
parameter is no longer needed -> toss it out

Signed-off-by: Jiri Pirko <jpirko@redhat.com>
Reviewed-by: Nicolas de Pesloüan <nicolas.2p.debian@free.fr>

v1->v2:
	moved __copy_skb_header hunk to 7/8

---
 drivers/block/aoe/aoenet.c      |    2 +-
 drivers/net/hamradio/bpqether.c |    4 ++--
 drivers/net/pppoe.c             |    4 ++--
 drivers/net/wan/hdlc.c          |    2 +-
 drivers/net/wan/lapbether.c     |    2 +-
 drivers/scsi/fcoe/fcoe.c        |   12 ++++--------
 include/linux/netdevice.h       |    3 +--
 include/net/ax25.h              |    3 ++-
 include/net/datalink.h          |    2 +-
 include/net/ip.h                |    2 +-
 include/net/ipv6.h              |    3 +--
 include/net/irda/irda.h         |    3 +--
 include/net/llc.h               |    8 +++-----
 include/net/p8022.h             |    3 +--
 include/net/psnap.h             |    3 +--
 include/net/x25.h               |    2 +-
 net/802/p8022.c                 |    3 +--
 net/802/psnap.c                 |    7 +++----
 net/802/stp.c                   |    2 +-
 net/8021q/vlan.h                |    2 +-
 net/8021q/vlan_dev.c            |    2 +-
 net/appletalk/aarp.c            |    2 +-
 net/appletalk/ddp.c             |    6 +++---
 net/ax25/ax25_in.c              |    2 +-
 net/batman-adv/hard-interface.c |    6 ++----
 net/caif/caif_dev.c             |    2 +-
 net/can/af_can.c                |    2 +-
 net/core/dev.c                  |   26 ++++++++++++--------------
 net/decnet/af_decnet.c          |    2 +-
 net/decnet/dn_route.c           |    2 +-
 net/dsa/tag_dsa.c               |    2 +-
 net/dsa/tag_edsa.c              |    2 +-
 net/dsa/tag_trailer.c           |    2 +-
 net/econet/af_econet.c          |    2 +-
 net/ieee802154/af_ieee802154.c  |    2 +-
 net/ipv4/arp.c                  |    2 +-
 net/ipv4/ip_input.c             |    2 +-
 net/ipv4/ipconfig.c             |   13 ++++++++-----
 net/ipv6/ip6_input.c            |    3 ++-
 net/ipx/af_ipx.c                |    3 ++-
 net/irda/irlap_frame.c          |    2 +-
 net/llc/llc_core.c              |    3 +--
 net/llc/llc_input.c             |    8 ++++----
 net/packet/af_packet.c          |    6 +++---
 net/phonet/af_phonet.c          |    3 +--
 net/tipc/eth_media.c            |    2 +-
 net/x25/x25_dev.c               |    2 +-
 47 files changed, 85 insertions(+), 98 deletions(-)

diff --git a/drivers/block/aoe/aoenet.c b/drivers/block/aoe/aoenet.c
index 4d3bc0d..c12e14b 100644
--- a/drivers/block/aoe/aoenet.c
+++ b/drivers/block/aoe/aoenet.c
@@ -99,7 +99,7 @@ aoenet_xmit(struct sk_buff_head *queue)
  * (1) len doesn't include the header by default.  I want this. 
  */
 static int
-aoenet_rcv(struct sk_buff *skb, struct net_device *ifp, struct packet_type *pt, struct net_device *orig_dev)
+aoenet_rcv(struct sk_buff *skb, struct net_device *ifp, struct packet_type *pt)
 {
 	struct aoe_hdr *h;
 	u32 n;
diff --git a/drivers/net/hamradio/bpqether.c b/drivers/net/hamradio/bpqether.c
index 8931168..f17ca33 100644
--- a/drivers/net/hamradio/bpqether.c
+++ b/drivers/net/hamradio/bpqether.c
@@ -95,7 +95,7 @@ static char bcast_addr[6]={0xFF,0xFF,0xFF,0xFF,0xFF,0xFF};
 
 static char bpq_eth_addr[6];
 
-static int bpq_rcv(struct sk_buff *, struct net_device *, struct packet_type *, struct net_device *);
+static int bpq_rcv(struct sk_buff *, struct net_device *, struct packet_type *);
 static int bpq_device_event(struct notifier_block *, unsigned long, void *);
 
 static struct packet_type bpq_packet_type __read_mostly = {
@@ -177,7 +177,7 @@ static inline int dev_is_ethdev(struct net_device *dev)
 /*
  *	Receive an AX.25 frame via an ethernet interface.
  */
-static int bpq_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *ptype, struct net_device *orig_dev)
+static int bpq_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *ptype)
 {
 	int len;
 	char * ptr;
diff --git a/drivers/net/pppoe.c b/drivers/net/pppoe.c
index 78c0e3c..b99cdaa 100644
--- a/drivers/net/pppoe.c
+++ b/drivers/net/pppoe.c
@@ -419,7 +419,7 @@ abort_kfree:
  *
  ***********************************************************************/
 static int pppoe_rcv(struct sk_buff *skb, struct net_device *dev,
-		     struct packet_type *pt, struct net_device *orig_dev)
+		     struct packet_type *pt)
 {
 	struct pppoe_hdr *ph;
 	struct pppox_sock *po;
@@ -467,7 +467,7 @@ out:
  *
  ***********************************************************************/
 static int pppoe_disc_rcv(struct sk_buff *skb, struct net_device *dev,
-			  struct packet_type *pt, struct net_device *orig_dev)
+			  struct packet_type *pt)
 
 {
 	struct pppoe_hdr *ph;
diff --git a/drivers/net/wan/hdlc.c b/drivers/net/wan/hdlc.c
index 5d4bb61..0ac4f27 100644
--- a/drivers/net/wan/hdlc.c
+++ b/drivers/net/wan/hdlc.c
@@ -53,7 +53,7 @@ int hdlc_change_mtu(struct net_device *dev, int new_mtu)
 }
 
 static int hdlc_rcv(struct sk_buff *skb, struct net_device *dev,
-		    struct packet_type *p, struct net_device *orig_dev)
+		    struct packet_type *p)
 {
 	struct hdlc_device *hdlc = dev_to_hdlc(dev);
 
diff --git a/drivers/net/wan/lapbether.c b/drivers/net/wan/lapbether.c
index 7f5bb91..993d71a 100644
--- a/drivers/net/wan/lapbether.c
+++ b/drivers/net/wan/lapbether.c
@@ -86,7 +86,7 @@ static __inline__ int dev_is_ethdev(struct net_device *dev)
 /*
  *	Receive a LAPB frame via an ethernet interface.
  */
-static int lapbeth_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *ptype, struct net_device *orig_dev)
+static int lapbeth_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *ptype)
 {
 	int len, err;
 	struct lapbethdev *lapbeth;
diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index 3becc6a..d7442b6 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -70,7 +70,7 @@ DEFINE_PER_CPU(struct fcoe_percpu_s, fcoe_percpu);
 static int fcoe_reset(struct Scsi_Host *);
 static int fcoe_xmit(struct fc_lport *, struct fc_frame *);
 static int fcoe_rcv(struct sk_buff *, struct net_device *,
-		    struct packet_type *, struct net_device *);
+		    struct packet_type *);
 static int fcoe_percpu_receive_thread(void *);
 static void fcoe_clean_pending_queue(struct fc_lport *);
 static void fcoe_percpu_clean(struct fc_lport *);
@@ -88,7 +88,7 @@ static struct fcoe_interface
 *fcoe_hostlist_lookup_port(const struct net_device *);
 
 static int fcoe_fip_recv(struct sk_buff *, struct net_device *,
-			 struct packet_type *, struct net_device *);
+			 struct packet_type *);
 
 static void fcoe_fip_send(struct fcoe_ctlr *, struct sk_buff *);
 static void fcoe_update_src_mac(struct fc_lport *, u8 *);
@@ -465,14 +465,11 @@ static inline void fcoe_interface_put(struct fcoe_interface *fcoe)
  * @skb:      The receive skb
  * @netdev:   The associated net device
  * @ptype:    The packet_type structure which was used to register this handler
- * @orig_dev: The original net_device the the skb was received on.
- *	      (in case dev is a bond)
  *
  * Returns: 0 for success
  */
 static int fcoe_fip_recv(struct sk_buff *skb, struct net_device *netdev,
-			 struct packet_type *ptype,
-			 struct net_device *orig_dev)
+			 struct packet_type *ptype)
 {
 	struct fcoe_interface *fcoe;
 
@@ -1228,7 +1225,6 @@ static int fcoe_cpu_callback(struct notifier_block *nfb,
  * @skb:    The received packet
  * @netdev: The net device that the packet was received on
  * @ptype:  The packet type context
- * @olddev: The last device net device
  *
  * This routine is called by NET_RX_SOFTIRQ. It receives a packet, builds a
  * FC frame and passes the frame to libfc.
@@ -1236,7 +1232,7 @@ static int fcoe_cpu_callback(struct notifier_block *nfb,
  * Returns: 0 for success
  */
 int fcoe_rcv(struct sk_buff *skb, struct net_device *netdev,
-	     struct packet_type *ptype, struct net_device *olddev)
+	     struct packet_type *ptype)
 {
 	struct fc_lport *lport;
 	struct fcoe_rcv_info *fr;
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index ff386a4..48a9638 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1456,8 +1456,7 @@ struct packet_type {
 	struct net_device	*dev;	/* NULL is wildcarded here	     */
 	int			(*func) (struct sk_buff *,
 					 struct net_device *,
-					 struct packet_type *,
-					 struct net_device *);
+					 struct packet_type *);
 	struct sk_buff		*(*gso_segment)(struct sk_buff *skb,
 						u32 features);
 	int			(*gso_send_check)(struct sk_buff *skb);
diff --git a/include/net/ax25.h b/include/net/ax25.h
index 206d222..fcbe6d8 100644
--- a/include/net/ax25.h
+++ b/include/net/ax25.h
@@ -362,7 +362,8 @@ extern int  ax25_protocol_is_registered(unsigned int);
 
 /* ax25_in.c */
 extern int  ax25_rx_iframe(ax25_cb *, struct sk_buff *);
-extern int  ax25_kiss_rcv(struct sk_buff *, struct net_device *, struct packet_type *, struct net_device *);
+extern int  ax25_kiss_rcv(struct sk_buff *, struct net_device *,
+			  struct packet_type *);
 
 /* ax25_ip.c */
 extern int ax25_hard_header(struct sk_buff *, struct net_device *,
diff --git a/include/net/datalink.h b/include/net/datalink.h
index deb7ca7..5797ba3 100644
--- a/include/net/datalink.h
+++ b/include/net/datalink.h
@@ -9,7 +9,7 @@ struct datalink_proto {
         unsigned short  header_length;
 
         int     (*rcvfunc)(struct sk_buff *, struct net_device *,
-                                struct packet_type *, struct net_device *);
+                                struct packet_type *);
 	int     (*request)(struct datalink_proto *, struct sk_buff *,
                                         unsigned char *);
 	struct list_head node;
diff --git a/include/net/ip.h b/include/net/ip.h
index a4f6311..3b04c24 100644
--- a/include/net/ip.h
+++ b/include/net/ip.h
@@ -94,7 +94,7 @@ extern int		ip_build_and_send_pkt(struct sk_buff *skb, struct sock *sk,
 					      __be32 saddr, __be32 daddr,
 					      struct ip_options *opt);
 extern int		ip_rcv(struct sk_buff *skb, struct net_device *dev,
-			       struct packet_type *pt, struct net_device *orig_dev);
+			       struct packet_type *pt);
 extern int		ip_local_deliver(struct sk_buff *skb);
 extern int		ip_mr_input(struct sk_buff *skb);
 extern int		ip_output(struct sk_buff *skb);
diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index d6d077d..990f90b 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -470,8 +470,7 @@ static __inline__ void ipv6_select_ident(struct frag_hdr *fhdr)
 
 extern int			ipv6_rcv(struct sk_buff *skb, 
 					 struct net_device *dev, 
-					 struct packet_type *pt,
-					 struct net_device *orig_dev);
+					 struct packet_type *pt);
 
 extern int			ip6_rcv_finish(struct sk_buff *skb);
 
diff --git a/include/net/irda/irda.h b/include/net/irda/irda.h
index 3bed61d..2c6e25d 100644
--- a/include/net/irda/irda.h
+++ b/include/net/irda/irda.h
@@ -125,7 +125,6 @@ extern int irda_nl_register(void);
 extern void irda_nl_unregister(void);
 
 extern int irlap_driver_rcv(struct sk_buff *skb, struct net_device *dev,
-			    struct packet_type *ptype,
-			    struct net_device *orig_dev);
+			    struct packet_type *ptype);
 
 #endif /* NET_IRDA_H */
diff --git a/include/net/llc.h b/include/net/llc.h
index 5503b74..73bfea5 100644
--- a/include/net/llc.h
+++ b/include/net/llc.h
@@ -58,8 +58,7 @@ struct llc_sap {
 	atomic_t         refcnt;
 	int		 (*rcv_func)(struct sk_buff *skb,
 				     struct net_device *dev,
-				     struct packet_type *pt,
-				     struct net_device *orig_dev);
+				     struct packet_type *pt);
 	struct llc_addr	 laddr;
 	struct list_head node;
 	spinlock_t sk_lock;
@@ -96,7 +95,7 @@ extern struct list_head llc_sap_list;
 extern spinlock_t llc_sap_list_lock;
 
 extern int llc_rcv(struct sk_buff *skb, struct net_device *dev,
-		   struct packet_type *pt, struct net_device *orig_dev);
+		   struct packet_type *pt);
 
 extern int llc_mac_hdr_init(struct sk_buff *skb,
 			    const unsigned char *sa, const unsigned char *da);
@@ -110,8 +109,7 @@ extern void llc_set_station_handler(void (*handler)(struct sk_buff *skb));
 extern struct llc_sap *llc_sap_open(unsigned char lsap,
 				    int (*rcv)(struct sk_buff *skb,
 					       struct net_device *dev,
-					       struct packet_type *pt,
-					       struct net_device *orig_dev));
+					       struct packet_type *pt));
 static inline void llc_sap_hold(struct llc_sap *sap)
 {
 	atomic_inc(&sap->refcnt);
diff --git a/include/net/p8022.h b/include/net/p8022.h
index 42e9fac..6d878ae 100644
--- a/include/net/p8022.h
+++ b/include/net/p8022.h
@@ -4,8 +4,7 @@ extern struct datalink_proto *
 	register_8022_client(unsigned char type,
 			     int (*func)(struct sk_buff *skb,
 					 struct net_device *dev,
-					 struct packet_type *pt,
-					 struct net_device *orig_dev));
+					 struct packet_type *pt));
 extern void unregister_8022_client(struct datalink_proto *proto);
 
 extern struct datalink_proto *make_8023_client(void);
diff --git a/include/net/psnap.h b/include/net/psnap.h
index fe456c2..604bf4b 100644
--- a/include/net/psnap.h
+++ b/include/net/psnap.h
@@ -4,8 +4,7 @@
 extern struct datalink_proto *
 register_snap_client(const unsigned char *desc,
 		     int (*rcvfunc)(struct sk_buff *, struct net_device *,
-				    struct packet_type *,
-				    struct net_device *orig_dev));
+				    struct packet_type *));
 extern void unregister_snap_client(struct datalink_proto *proto);
 
 #endif
diff --git a/include/net/x25.h b/include/net/x25.h
index a06119a..3f1c0df2 100644
--- a/include/net/x25.h
+++ b/include/net/x25.h
@@ -202,7 +202,7 @@ extern void x25_kill_by_neigh(struct x25_neigh *);
 
 /* x25_dev.c */
 extern void x25_send_frame(struct sk_buff *, struct x25_neigh *);
-extern int  x25_lapb_receive_frame(struct sk_buff *, struct net_device *, struct packet_type *, struct net_device *);
+extern int  x25_lapb_receive_frame(struct sk_buff *, struct net_device *, struct packet_type *);
 extern void x25_establish_link(struct x25_neigh *);
 extern void x25_terminate_link(struct x25_neigh *);
 
diff --git a/net/802/p8022.c b/net/802/p8022.c
index 7f353c4..4ab1339 100644
--- a/net/802/p8022.c
+++ b/net/802/p8022.c
@@ -36,8 +36,7 @@ static int p8022_request(struct datalink_proto *dl, struct sk_buff *skb,
 struct datalink_proto *register_8022_client(unsigned char type,
 					    int (*func)(struct sk_buff *skb,
 							struct net_device *dev,
-							struct packet_type *pt,
-							struct net_device *orig_dev))
+							struct packet_type *pt))
 {
 	struct datalink_proto *proto;
 
diff --git a/net/802/psnap.c b/net/802/psnap.c
index 21cde8f..e74b48e 100644
--- a/net/802/psnap.c
+++ b/net/802/psnap.c
@@ -47,7 +47,7 @@ static struct datalink_proto *find_snap_client(const unsigned char *desc)
  *	A SNAP packet has arrived
  */
 static int snap_rcv(struct sk_buff *skb, struct net_device *dev,
-		    struct packet_type *pt, struct net_device *orig_dev)
+		    struct packet_type *pt)
 {
 	int rc = 1;
 	struct datalink_proto *proto;
@@ -64,7 +64,7 @@ static int snap_rcv(struct sk_buff *skb, struct net_device *dev,
 		/* Pass the frame on. */
 		skb->transport_header += 5;
 		skb_pull_rcsum(skb, 5);
-		rc = proto->rcvfunc(skb, dev, &snap_packet_type, orig_dev);
+		rc = proto->rcvfunc(skb, dev, &snap_packet_type);
 	}
 	rcu_read_unlock();
 
@@ -126,8 +126,7 @@ module_exit(snap_exit);
 struct datalink_proto *register_snap_client(const unsigned char *desc,
 					    int (*rcvfunc)(struct sk_buff *,
 							   struct net_device *,
-							   struct packet_type *,
-							   struct net_device *))
+							   struct packet_type *))
 {
 	struct datalink_proto *proto = NULL;
 
diff --git a/net/802/stp.c b/net/802/stp.c
index 978c30b..dec1f88 100644
--- a/net/802/stp.c
+++ b/net/802/stp.c
@@ -30,7 +30,7 @@ static DEFINE_MUTEX(stp_proto_mutex);
 
 /* Called under rcu_read_lock from LLC */
 static int stp_pdu_rcv(struct sk_buff *skb, struct net_device *dev,
-		       struct packet_type *pt, struct net_device *orig_dev)
+		       struct packet_type *pt)
 {
 	const struct ethhdr *eh = eth_hdr(skb);
 	const struct llc_pdu_un *pdu = llc_pdu_un_hdr(skb);
diff --git a/net/8021q/vlan.h b/net/8021q/vlan.h
index 5687c9b..6f8941e 100644
--- a/net/8021q/vlan.h
+++ b/net/8021q/vlan.h
@@ -76,7 +76,7 @@ static inline struct vlan_dev_info *vlan_dev_info(const struct net_device *dev)
 
 /* found in vlan_dev.c */
 int vlan_skb_recv(struct sk_buff *skb, struct net_device *dev,
-		  struct packet_type *ptype, struct net_device *orig_dev);
+		  struct packet_type *ptype);
 void vlan_dev_set_ingress_priority(const struct net_device *dev,
 				   u32 skb_prio, u16 vlan_prio);
 int vlan_dev_set_egress_priority(const struct net_device *dev,
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index be73753..6e396b9 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -138,7 +138,7 @@ static inline void vlan_set_encap_proto(struct sk_buff *skb,
  *
  */
 int vlan_skb_recv(struct sk_buff *skb, struct net_device *dev,
-		  struct packet_type *ptype, struct net_device *orig_dev)
+		  struct packet_type *ptype)
 {
 	struct vlan_hdr *vhdr;
 	struct vlan_pcpu_stats *rx_stats;
diff --git a/net/appletalk/aarp.c b/net/appletalk/aarp.c
index 50dce79..996f366 100644
--- a/net/appletalk/aarp.c
+++ b/net/appletalk/aarp.c
@@ -715,7 +715,7 @@ static void __aarp_resolved(struct aarp_entry **list, struct aarp_entry *a,
  *	frame. We currently only support Ethernet.
  */
 static int aarp_rcv(struct sk_buff *skb, struct net_device *dev,
-		    struct packet_type *pt, struct net_device *orig_dev)
+		    struct packet_type *pt)
 {
 	struct elapaarp *ea = aarp_hdr(skb);
 	int hash, ret = 0;
diff --git a/net/appletalk/ddp.c b/net/appletalk/ddp.c
index c410b93..61b4983 100644
--- a/net/appletalk/ddp.c
+++ b/net/appletalk/ddp.c
@@ -1425,7 +1425,7 @@ drop:
  *	layer.  [ie ARPHRD_ETHERTALK]
  */
 static int atalk_rcv(struct sk_buff *skb, struct net_device *dev,
-		     struct packet_type *pt, struct net_device *orig_dev)
+		     struct packet_type *pt)
 {
 	struct ddpehdr *ddp;
 	struct sock *sock;
@@ -1522,7 +1522,7 @@ out:
  * header and append a long one.
  */
 static int ltalk_rcv(struct sk_buff *skb, struct net_device *dev,
-		     struct packet_type *pt, struct net_device *orig_dev)
+		     struct packet_type *pt)
 {
 	if (!net_eq(dev_net(dev), &init_net))
 		goto freeit;
@@ -1568,7 +1568,7 @@ static int ltalk_rcv(struct sk_buff *skb, struct net_device *dev,
 	}
 	skb_reset_transport_header(skb);
 
-	return atalk_rcv(skb, dev, pt, orig_dev);
+	return atalk_rcv(skb, dev, pt);
 freeit:
 	kfree_skb(skb);
 	return 0;
diff --git a/net/ax25/ax25_in.c b/net/ax25/ax25_in.c
index 9bb7765..f6f94e6 100644
--- a/net/ax25/ax25_in.c
+++ b/net/ax25/ax25_in.c
@@ -436,7 +436,7 @@ free:
  *	Receive an AX.25 frame via a SLIP interface.
  */
 int ax25_kiss_rcv(struct sk_buff *skb, struct net_device *dev,
-		  struct packet_type *ptype, struct net_device *orig_dev)
+		  struct packet_type *ptype)
 {
 	skb_orphan(skb);
 
diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c
index f2131f4..08579b0 100644
--- a/net/batman-adv/hard-interface.c
+++ b/net/batman-adv/hard-interface.c
@@ -37,8 +37,7 @@ static DEFINE_SPINLOCK(if_list_lock);
 
 static int batman_skb_recv(struct sk_buff *skb,
 			   struct net_device *dev,
-			   struct packet_type *ptype,
-			   struct net_device *orig_dev);
+			   struct packet_type *ptype);
 
 static void hardif_free_rcu(struct rcu_head *rcu)
 {
@@ -556,8 +555,7 @@ out:
 /* receive a packet with the batman ethertype coming on a hard
  * interface */
 static int batman_skb_recv(struct sk_buff *skb, struct net_device *dev,
-			   struct packet_type *ptype,
-			   struct net_device *orig_dev)
+			   struct packet_type *ptype)
 {
 	struct bat_priv *bat_priv;
 	struct batman_packet *batman_packet;
diff --git a/net/caif/caif_dev.c b/net/caif/caif_dev.c
index a42a408..9f016f0 100644
--- a/net/caif/caif_dev.c
+++ b/net/caif/caif_dev.c
@@ -165,7 +165,7 @@ static int modemcmd(struct cflayer *layr, enum caif_modemcmd ctrl)
  * On error, returns non-zero and releases the skb.
  */
 static int receive(struct sk_buff *skb, struct net_device *dev,
-		   struct packet_type *pkttype, struct net_device *orig_dev)
+		   struct packet_type *pkttype)
 {
 	struct net *net;
 	struct cfpkt *pkt;
diff --git a/net/can/af_can.c b/net/can/af_can.c
index 702be5a..ad32de5 100644
--- a/net/can/af_can.c
+++ b/net/can/af_can.c
@@ -616,7 +616,7 @@ static int can_rcv_filter(struct dev_rcv_lists *d, struct sk_buff *skb)
 }
 
 static int can_rcv(struct sk_buff *skb, struct net_device *dev,
-		   struct packet_type *pt, struct net_device *orig_dev)
+		   struct packet_type *pt)
 {
 	struct dev_rcv_lists *d;
 	struct can_frame *cf = (struct can_frame *)skb->data;
diff --git a/net/core/dev.c b/net/core/dev.c
index 58daddb..c71bd18 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1534,11 +1534,10 @@ int dev_forward_skb(struct net_device *dev, struct sk_buff *skb)
 EXPORT_SYMBOL_GPL(dev_forward_skb);
 
 static inline int deliver_skb(struct sk_buff *skb,
-			      struct packet_type *pt_prev,
-			      struct net_device *orig_dev)
+			      struct packet_type *pt_prev)
 {
 	atomic_inc(&skb->users);
-	return pt_prev->func(skb, skb->dev, pt_prev, orig_dev);
+	return pt_prev->func(skb, skb->dev, pt_prev);
 }
 
 /*
@@ -1561,7 +1560,7 @@ static void dev_queue_xmit_nit(struct sk_buff *skb, struct net_device *dev)
 		    (ptype->af_packet_priv == NULL ||
 		     (struct sock *)ptype->af_packet_priv != skb->sk)) {
 			if (pt_prev) {
-				deliver_skb(skb2, pt_prev, skb->dev);
+				deliver_skb(skb2, pt_prev);
 				pt_prev = ptype;
 				continue;
 			}
@@ -1594,7 +1593,7 @@ static void dev_queue_xmit_nit(struct sk_buff *skb, struct net_device *dev)
 		}
 	}
 	if (pt_prev)
-		pt_prev->func(skb2, skb->dev, pt_prev, skb->dev);
+		pt_prev->func(skb2, skb->dev, pt_prev);
 	rcu_read_unlock();
 }
 
@@ -3025,8 +3024,7 @@ static int ing_filter(struct sk_buff *skb, struct netdev_queue *rxq)
 }
 
 static inline struct sk_buff *handle_ing(struct sk_buff *skb,
-					 struct packet_type **pt_prev,
-					 int *ret, struct net_device *orig_dev)
+					 struct packet_type **pt_prev, int *ret)
 {
 	struct netdev_queue *rxq = rcu_dereference(skb->dev->ingress_queue);
 
@@ -3034,7 +3032,7 @@ static inline struct sk_buff *handle_ing(struct sk_buff *skb,
 		goto out;
 
 	if (*pt_prev) {
-		*ret = deliver_skb(skb, *pt_prev, orig_dev);
+		*ret = deliver_skb(skb, *pt_prev);
 		*pt_prev = NULL;
 	}
 
@@ -3160,13 +3158,13 @@ another_round:
 	list_for_each_entry_rcu(ptype, &ptype_all, list) {
 		if (!ptype->dev || ptype->dev == skb->dev) {
 			if (pt_prev)
-				ret = deliver_skb(skb, pt_prev, orig_dev);
+				ret = deliver_skb(skb, pt_prev);
 			pt_prev = ptype;
 		}
 	}
 
 #ifdef CONFIG_NET_CLS_ACT
-	skb = handle_ing(skb, &pt_prev, &ret, orig_dev);
+	skb = handle_ing(skb, &pt_prev, &ret);
 	if (!skb)
 		goto out;
 ncls:
@@ -3175,7 +3173,7 @@ ncls:
 	rx_handler = rcu_dereference(skb->dev->rx_handler);
 	if (rx_handler) {
 		if (pt_prev) {
-			ret = deliver_skb(skb, pt_prev, orig_dev);
+			ret = deliver_skb(skb, pt_prev);
 			pt_prev = NULL;
 		}
 		switch (rx_handler(&skb)) {
@@ -3194,7 +3192,7 @@ ncls:
 
 	if (vlan_tx_tag_present(skb)) {
 		if (pt_prev) {
-			ret = deliver_skb(skb, pt_prev, orig_dev);
+			ret = deliver_skb(skb, pt_prev);
 			pt_prev = NULL;
 		}
 		if (vlan_hwaccel_do_receive(&skb)) {
@@ -3216,13 +3214,13 @@ ncls:
 		    (ptype->dev == null_or_dev || ptype->dev == skb->dev ||
 		     ptype->dev == orig_dev)) {
 			if (pt_prev)
-				ret = deliver_skb(skb, pt_prev, orig_dev);
+				ret = deliver_skb(skb, pt_prev);
 			pt_prev = ptype;
 		}
 	}
 
 	if (pt_prev) {
-		ret = pt_prev->func(skb, skb->dev, pt_prev, orig_dev);
+		ret = pt_prev->func(skb, skb->dev, pt_prev);
 	} else {
 		atomic_long_inc(&skb->dev->rx_dropped);
 		kfree_skb(skb);
diff --git a/net/decnet/af_decnet.c b/net/decnet/af_decnet.c
index 2af15b1..1421721 100644
--- a/net/decnet/af_decnet.c
+++ b/net/decnet/af_decnet.c
@@ -2106,7 +2106,7 @@ static struct notifier_block dn_dev_notifier = {
 	.notifier_call = dn_device_event,
 };
 
-extern int dn_route_rcv(struct sk_buff *, struct net_device *, struct packet_type *, struct net_device *);
+extern int dn_route_rcv(struct sk_buff *, struct net_device *, struct packet_type *);
 
 static struct packet_type dn_dix_packet_type __read_mostly = {
 	.type =		cpu_to_be16(ETH_P_DNA_RT),
diff --git a/net/decnet/dn_route.c b/net/decnet/dn_route.c
index 484fdbf..20abc79 100644
--- a/net/decnet/dn_route.c
+++ b/net/decnet/dn_route.c
@@ -588,7 +588,7 @@ static int dn_route_ptp_hello(struct sk_buff *skb)
 	return NET_RX_SUCCESS;
 }
 
-int dn_route_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *pt, struct net_device *orig_dev)
+int dn_route_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *pt)
 {
 	struct dn_skb_cb *cb;
 	unsigned char flags = 0;
diff --git a/net/dsa/tag_dsa.c b/net/dsa/tag_dsa.c
index 98dfe80..56b686e 100644
--- a/net/dsa/tag_dsa.c
+++ b/net/dsa/tag_dsa.c
@@ -77,7 +77,7 @@ out_free:
 }
 
 static int dsa_rcv(struct sk_buff *skb, struct net_device *dev,
-		   struct packet_type *pt, struct net_device *orig_dev)
+		   struct packet_type *pt)
 {
 	struct dsa_switch_tree *dst = dev->dsa_ptr;
 	struct dsa_switch *ds;
diff --git a/net/dsa/tag_edsa.c b/net/dsa/tag_edsa.c
index 6f38332..474a955 100644
--- a/net/dsa/tag_edsa.c
+++ b/net/dsa/tag_edsa.c
@@ -90,7 +90,7 @@ out_free:
 }
 
 static int edsa_rcv(struct sk_buff *skb, struct net_device *dev,
-		    struct packet_type *pt, struct net_device *orig_dev)
+		    struct packet_type *pt)
 {
 	struct dsa_switch_tree *dst = dev->dsa_ptr;
 	struct dsa_switch *ds;
diff --git a/net/dsa/tag_trailer.c b/net/dsa/tag_trailer.c
index d6d7d0a..f8f36ad 100644
--- a/net/dsa/tag_trailer.c
+++ b/net/dsa/tag_trailer.c
@@ -67,7 +67,7 @@ netdev_tx_t trailer_xmit(struct sk_buff *skb, struct net_device *dev)
 }
 
 static int trailer_rcv(struct sk_buff *skb, struct net_device *dev,
-		       struct packet_type *pt, struct net_device *orig_dev)
+		       struct packet_type *pt)
 {
 	struct dsa_switch_tree *dst = dev->dsa_ptr;
 	struct dsa_switch *ds;
diff --git a/net/econet/af_econet.c b/net/econet/af_econet.c
index 0c28263..edeea09 100644
--- a/net/econet/af_econet.c
+++ b/net/econet/af_econet.c
@@ -1052,7 +1052,7 @@ release:
  *	Receive an Econet frame from a device.
  */
 
-static int econet_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *pt, struct net_device *orig_dev)
+static int econet_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *pt)
 {
 	struct ec_framehdr *hdr;
 	struct sock *sk = NULL;
diff --git a/net/ieee802154/af_ieee802154.c b/net/ieee802154/af_ieee802154.c
index 6df6ecf..68d5bce 100644
--- a/net/ieee802154/af_ieee802154.c
+++ b/net/ieee802154/af_ieee802154.c
@@ -299,7 +299,7 @@ static const struct net_proto_family ieee802154_family_ops = {
 };
 
 static int ieee802154_rcv(struct sk_buff *skb, struct net_device *dev,
-	struct packet_type *pt, struct net_device *orig_dev)
+			  struct packet_type *pt)
 {
 	if (!netif_running(dev))
 		return -ENODEV;
diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index fa9988d..53a8f14 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -974,7 +974,7 @@ static void parp_redo(struct sk_buff *skb)
  */
 
 static int arp_rcv(struct sk_buff *skb, struct net_device *dev,
-		   struct packet_type *pt, struct net_device *orig_dev)
+		   struct packet_type *pt)
 {
 	struct arphdr *arp;
 
diff --git a/net/ipv4/ip_input.c b/net/ipv4/ip_input.c
index d7b2b09..639b69b 100644
--- a/net/ipv4/ip_input.c
+++ b/net/ipv4/ip_input.c
@@ -372,7 +372,7 @@ drop:
 /*
  * 	Main IP Receive routine.
  */
-int ip_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *pt, struct net_device *orig_dev)
+int ip_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *pt)
 {
 	struct iphdr *iph;
 	u32 len;
diff --git a/net/ipv4/ipconfig.c b/net/ipv4/ipconfig.c
index 2b09775..5669509 100644
--- a/net/ipv4/ipconfig.c
+++ b/net/ipv4/ipconfig.c
@@ -438,7 +438,8 @@ static int __init ic_defaults(void)
 
 #ifdef IPCONFIG_RARP
 
-static int ic_rarp_recv(struct sk_buff *skb, struct net_device *dev, struct packet_type *pt, struct net_device *orig_dev);
+static int ic_rarp_recv(struct sk_buff *skb, struct net_device *dev,
+			struct packet_type *pt);
 
 static struct packet_type rarp_packet_type __initdata = {
 	.type =	cpu_to_be16(ETH_P_RARP),
@@ -458,8 +459,8 @@ static inline void __init ic_rarp_cleanup(void)
 /*
  *  Process received RARP packet.
  */
-static int __init
-ic_rarp_recv(struct sk_buff *skb, struct net_device *dev, struct packet_type *pt, struct net_device *orig_dev)
+static int __init ic_rarp_recv(struct sk_buff *skb, struct net_device *dev,
+			       struct packet_type *pt)
 {
 	struct arphdr *rarp;
 	unsigned char *rarp_ptr;
@@ -600,7 +601,8 @@ struct bootp_pkt {		/* BOOTP packet format */
 #define DHCPRELEASE	7
 #define DHCPINFORM	8
 
-static int ic_bootp_recv(struct sk_buff *skb, struct net_device *dev, struct packet_type *pt, struct net_device *orig_dev);
+static int ic_bootp_recv(struct sk_buff *skb, struct net_device *dev
+			 struct packet_type *pt);
 
 static struct packet_type bootp_packet_type __initdata = {
 	.type =	cpu_to_be16(ETH_P_IP),
@@ -893,7 +895,8 @@ static void __init ic_do_bootp_ext(u8 *ext)
 /*
  *  Receive BOOTP reply.
  */
-static int __init ic_bootp_recv(struct sk_buff *skb, struct net_device *dev, struct packet_type *pt, struct net_device *orig_dev)
+static int __init ic_bootp_recv(struct sk_buff *skb, struct net_device *dev,
+				struct packet_type *pt)
 {
 	struct bootp_pkt *b;
 	struct iphdr *h;
diff --git a/net/ipv6/ip6_input.c b/net/ipv6/ip6_input.c
index a83e920..dd72591 100644
--- a/net/ipv6/ip6_input.c
+++ b/net/ipv6/ip6_input.c
@@ -55,7 +55,8 @@ inline int ip6_rcv_finish( struct sk_buff *skb)
 	return dst_input(skb);
 }
 
-int ipv6_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *pt, struct net_device *orig_dev)
+int ipv6_rcv(struct sk_buff *skb, struct net_device *dev,
+	     struct packet_type *pt)
 {
 	struct ipv6hdr *hdr;
 	u32 		pkt_len;
diff --git a/net/ipx/af_ipx.c b/net/ipx/af_ipx.c
index da3d21c..614e207 100644
--- a/net/ipx/af_ipx.c
+++ b/net/ipx/af_ipx.c
@@ -1665,7 +1665,8 @@ static unsigned int ipx_datagram_poll(struct file *file, struct socket *sock,
 	return rc;
 }
 
-static int ipx_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *pt, struct net_device *orig_dev)
+static int ipx_rcv(struct sk_buff *skb, struct net_device *dev,
+		   struct packet_type *pt)
 {
 	/* NULL here for pt means the packet was looped back */
 	struct ipx_interface *intrfc;
diff --git a/net/irda/irlap_frame.c b/net/irda/irlap_frame.c
index 688222c..7850321 100644
--- a/net/irda/irlap_frame.c
+++ b/net/irda/irlap_frame.c
@@ -1306,7 +1306,7 @@ static void irlap_recv_test_frame(struct irlap_cb *self, struct sk_buff *skb,
  * Jean II
  */
 int irlap_driver_rcv(struct sk_buff *skb, struct net_device *dev,
-		     struct packet_type *ptype, struct net_device *orig_dev)
+		     struct packet_type *ptype)
 {
 	struct irlap_info info;
 	struct irlap_cb *self;
diff --git a/net/llc/llc_core.c b/net/llc/llc_core.c
index 2bb0ddf..dfeda38 100644
--- a/net/llc/llc_core.c
+++ b/net/llc/llc_core.c
@@ -91,8 +91,7 @@ struct llc_sap *llc_sap_find(unsigned char sap_value)
 struct llc_sap *llc_sap_open(unsigned char lsap,
 			     int (*func)(struct sk_buff *skb,
 					 struct net_device *dev,
-					 struct packet_type *pt,
-					 struct net_device *orig_dev))
+					 struct packet_type *pt))
 {
 	struct llc_sap *sap = NULL;
 
diff --git a/net/llc/llc_input.c b/net/llc/llc_input.c
index 058f1e9..d884e23 100644
--- a/net/llc/llc_input.c
+++ b/net/llc/llc_input.c
@@ -143,13 +143,13 @@ static inline int llc_fixup_skb(struct sk_buff *skb)
  *	data now), it queues this frame in the connection's backlog.
  */
 int llc_rcv(struct sk_buff *skb, struct net_device *dev,
-	    struct packet_type *pt, struct net_device *orig_dev)
+	    struct packet_type *pt)
 {
 	struct llc_sap *sap;
 	struct llc_pdu_sn *pdu;
 	int dest;
 	int (*rcv)(struct sk_buff *, struct net_device *,
-		   struct packet_type *, struct net_device *);
+		   struct packet_type *);
 
 	if (!net_eq(dev_net(dev), &init_net))
 		goto drop;
@@ -184,14 +184,14 @@ int llc_rcv(struct sk_buff *skb, struct net_device *dev,
 	dest = llc_pdu_type(skb);
 	if (unlikely(!dest || !llc_type_handlers[dest - 1])) {
 		if (rcv)
-			rcv(skb, dev, pt, orig_dev);
+			rcv(skb, dev, pt);
 		else
 			kfree_skb(skb);
 	} else {
 		if (rcv) {
 			struct sk_buff *cskb = skb_clone(skb, GFP_ATOMIC);
 			if (cskb)
-				rcv(cskb, dev, pt, orig_dev);
+				rcv(cskb, dev, pt);
 		}
 		llc_type_handlers[dest - 1](sap, skb);
 	}
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index b34294e..85e2493 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -350,7 +350,7 @@ static const struct proto_ops packet_ops;
 static const struct proto_ops packet_ops_spkt;
 
 static int packet_rcv_spkt(struct sk_buff *skb, struct net_device *dev,
-			   struct packet_type *pt, struct net_device *orig_dev)
+			   struct packet_type *pt)
 {
 	struct sock *sk;
 	struct sockaddr_pkt *spkt;
@@ -557,7 +557,7 @@ static inline unsigned int run_filter(const struct sk_buff *skb,
  */
 
 static int packet_rcv(struct sk_buff *skb, struct net_device *dev,
-		      struct packet_type *pt, struct net_device *orig_dev)
+		      struct packet_type *pt)
 {
 	struct sock *sk;
 	struct sockaddr_ll *sll;
@@ -667,7 +667,7 @@ drop:
 }
 
 static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev,
-		       struct packet_type *pt, struct net_device *orig_dev)
+		       struct packet_type *pt)
 {
 	struct sock *sk;
 	struct packet_sock *po;
diff --git a/net/phonet/af_phonet.c b/net/phonet/af_phonet.c
index 30cc676..2fb5bf2 100644
--- a/net/phonet/af_phonet.c
+++ b/net/phonet/af_phonet.c
@@ -372,8 +372,7 @@ static int send_reset_indications(struct sk_buff *rskb)
  * On error, returns non-zero and releases the skb.
  */
 static int phonet_rcv(struct sk_buff *skb, struct net_device *dev,
-			struct packet_type *pkttype,
-			struct net_device *orig_dev)
+			struct packet_type *pkttype)
 {
 	struct net *net = dev_net(dev);
 	struct phonethdr *ph;
diff --git a/net/tipc/eth_media.c b/net/tipc/eth_media.c
index b69092e..21c8fe4 100644
--- a/net/tipc/eth_media.c
+++ b/net/tipc/eth_media.c
@@ -100,7 +100,7 @@ static int send_msg(struct sk_buff *buf, struct tipc_bearer *tb_ptr,
  */
 
 static int recv_msg(struct sk_buff *buf, struct net_device *dev,
-		    struct packet_type *pt, struct net_device *orig_dev)
+		    struct packet_type *pt)
 {
 	struct eth_bearer *eb_ptr = (struct eth_bearer *)pt->af_packet_priv;
 
diff --git a/net/x25/x25_dev.c b/net/x25/x25_dev.c
index 9005f6d..4b049ec 100644
--- a/net/x25/x25_dev.c
+++ b/net/x25/x25_dev.c
@@ -92,7 +92,7 @@ static int x25_receive_data(struct sk_buff *skb, struct x25_neigh *nb)
 }
 
 int x25_lapb_receive_frame(struct sk_buff *skb, struct net_device *dev,
-			   struct packet_type *ptype, struct net_device *orig_dev)
+			   struct packet_type *ptype)
 {
 	struct sk_buff *nskb;
 	struct x25_neigh *nb;
-- 
1.7.4


^ permalink raw reply related

* Re: [patch net-next-2.6] net: comment rx_handler results
From: Jiri Pirko @ 2011-03-05 15:37 UTC (permalink / raw)
  To: Nicolas de Pesloüan
  Cc: Ben Hutchings, netdev, davem, shemminger, kaber, fubar,
	eric.dumazet, andy
In-Reply-To: <4D725678.2020706@gmail.com>

Sat, Mar 05, 2011 at 04:27:52PM CET, nicolas.2p.debian@gmail.com wrote:
>Le 05/03/2011 16:13, Jiri Pirko a écrit :
>>
>>Signed-off-by: Jiri Pirko<jpirko@redhat.com>
>>---
>>  include/linux/netdevice.h |   12 ++++++++----
>>  1 files changed, 8 insertions(+), 4 deletions(-)
>>
>>diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>>index 48a9638..26e03f9 100644
>>--- a/include/linux/netdevice.h
>>+++ b/include/linux/netdevice.h
>>@@ -391,10 +391,14 @@ enum gro_result {
>>  typedef enum gro_result gro_result_t;
>>
>>  enum rx_handler_result {
>>-	RX_HANDLER_CONSUMED,
>>-	RX_HANDLER_ANOTHER,
>>-	RX_HANDLER_EXACT,
>>-	RX_HANDLER_PASS,
>>+	RX_HANDLER_CONSUMED,	/* skb was consumed by rx_handler,
>>+				   do not process it further. */
>>+	RX_HANDLER_ANOTHER,	/* Do another round in receive path.
>>+				   This is indicated in case skb->dev
>>+				   was changed by rx_handler */
>>+	RX_HANDLER_EXACT,	/* Force exact delivery, no wildcard */
>>+	RX_HANDLER_PASS,	/* Do nothing, pass the skb as if
>>+				   no rx_handler was called */
>
>Changing skb->dev should always cause another_round.

Not if you reinject the skb.

>
>So, RX_HANDLER_ANOTHER could be removed and replaced with a test for
>skb->dev change, with RX_HANDER_EXACT and RX_HANDLER_PASS.

That was there before. Needs extra variable and I think this way, via
results the code is much easier to read.

>
>> +	RX_HANDLER_CONSUMED,	/* skb was consumed by rx_handler,
>> +				   do not process it further. */
>> +	RX_HANDLER_EXACT,	/* Force exact delivery, no wildcard */
>> +	RX_HANDLER_PASS,	/* Do normal (wildcard and exact) delivery */
>
>And any rx_handler returning RX_HANDLER_EXACT should cause exact
>delivery, even if possible next rx_handlers return RX_HANDLER_PASS.

When rx_handler returns RX_HANDLER_EXACT, there will bo no other call to
any rx_handler (that is the case only when RX_HANDLER_ANOTHER is
returned)
>
>	Nicolas.

^ permalink raw reply

* Re: [patch net-next-2.6] net: comment rx_handler results
From: Nicolas de Pesloüan @ 2011-03-05 15:50 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: Ben Hutchings, netdev, davem, shemminger, kaber, fubar,
	eric.dumazet, andy
In-Reply-To: <20110305153704.GI8573@psychotron.redhat.com>

Le 05/03/2011 16:37, Jiri Pirko a écrit :
> Sat, Mar 05, 2011 at 04:27:52PM CET, nicolas.2p.debian@gmail.com wrote:
>> Le 05/03/2011 16:13, Jiri Pirko a écrit :
>>>
>>> Signed-off-by: Jiri Pirko<jpirko@redhat.com>
>>> ---
>>>   include/linux/netdevice.h |   12 ++++++++----
>>>   1 files changed, 8 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>>> index 48a9638..26e03f9 100644
>>> --- a/include/linux/netdevice.h
>>> +++ b/include/linux/netdevice.h
>>> @@ -391,10 +391,14 @@ enum gro_result {
>>>   typedef enum gro_result gro_result_t;
>>>
>>>   enum rx_handler_result {
>>> -	RX_HANDLER_CONSUMED,
>>> -	RX_HANDLER_ANOTHER,
>>> -	RX_HANDLER_EXACT,
>>> -	RX_HANDLER_PASS,
>>> +	RX_HANDLER_CONSUMED,	/* skb was consumed by rx_handler,
>>> +				   do not process it further. */
>>> +	RX_HANDLER_ANOTHER,	/* Do another round in receive path.
>>> +				   This is indicated in case skb->dev
>>> +				   was changed by rx_handler */
>>> +	RX_HANDLER_EXACT,	/* Force exact delivery, no wildcard */
>>> +	RX_HANDLER_PASS,	/* Do nothing, pass the skb as if
>>> +				   no rx_handler was called */
>>
>> Changing skb->dev should always cause another_round.
>
> Not if you reinject the skb.
>
>>
>> So, RX_HANDLER_ANOTHER could be removed and replaced with a test for
>> skb->dev change, with RX_HANDER_EXACT and RX_HANDLER_PASS.
>
> That was there before. Needs extra variable and I think this way, via
> results the code is much easier to read.
>
>>
>>> +	RX_HANDLER_CONSUMED,	/* skb was consumed by rx_handler,
>>> +				   do not process it further. */
>>> +	RX_HANDLER_EXACT,	/* Force exact delivery, no wildcard */
>>> +	RX_HANDLER_PASS,	/* Do normal (wildcard and exact) delivery */
>>
>> And any rx_handler returning RX_HANDLER_EXACT should cause exact
>> delivery, even if possible next rx_handlers return RX_HANDLER_PASS.
>
> When rx_handler returns RX_HANDLER_EXACT, there will bo no other call to
> any rx_handler (that is the case only when RX_HANDLER_ANOTHER is
> returned)

I agree, but we clearly need a kernel-doc that explain in more detail the expected behavior of every 
return values.

Some examples, showing some normal and some stacking setups, would help, I think. One more follow-up 
patch...

Reviewed-by: Nicolas de Pesloüan <nicolas.2p.debian@free.fr>

	Nicolas.

^ permalink raw reply

* Re: [PATCH] net: mac80211: fix compilation warning
From: Larry Finger @ 2011-03-05 16:30 UTC (permalink / raw)
  To: Jovi Zhang
  Cc: Ben Hutchings, John W. Linville, Johannes Berg, David S. Miller,
	open list:NETWORKING [WIREL..., open list:NETWORKING [GENERAL],
	open list
In-Reply-To: <AANLkTimnSvBKSL6_d3RxufxUL_C9wCM+Fz66_B5Sc0KE@mail.gmail.com>

On 03/05/2011 07:52 AM, Jovi Zhang wrote:
> On Sat, Mar 5, 2011 at 8:31 PM, Ben Hutchings<bhutchings@solarflare.com>  wrote:
>> On Wed, 2011-03-02 at 18:32 -0500, bookjovi@gmail.com wrote:
>>> From: Jovi Zhang<bookjovi@gmail.com>
>>>
>>> this commit fix compilation warning as following:
>>> net/mac80211/tx.c:1753: warning: unused variable mppath
>> [...]
>>
>> You clearly didn't try building this with CONFIG_MAC80211_MESH enabled.
>>
> Sorry, indeed, maybe should be like this:
>
> +#ifdef CONFIG_MAC80211_MESH
>         struct mesh_path *mppath = NULL;
> +#endif

Linville likes "struct mesh_path *mppath __maybe_unused = NULL;" over the ifdef 
form.

Larry

^ permalink raw reply

* Re: [patch net-next-2.6 1/8] af_packet: use skb->skb_iif instead of orig_dev->ifindex
From: Changli Gao @ 2011-03-05 16:34 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: netdev, davem, shemminger, kaber, fubar, eric.dumazet,
	nicolas.2p.debian, andy
In-Reply-To: <1299313794-5218-2-git-send-email-jpirko@redhat.com>

On Sat, Mar 5, 2011 at 4:29 PM, Jiri Pirko <jpirko@redhat.com> wrote:
> Since skb_iif has the desired value (ifindex of physical device actually
> received the traffic) use that instead.
>

It isn't true for the packets returned from a ifb pseudo NIC.

f.e.:

eth0 -(redirect)-> ifb0 ->

skb->skb_iff is ifb0->ifindex.
orig_dev = skb->dev = eth0.

However, these packets aren't processed by af_packet, so there isn't
any problem.

Reviewed-by: Changli Gao <xiaosuo@gmail.com>

-- 
Regards,
Changli Gao(xiaosuo@gmail.com)

^ permalink raw reply

* Re: [patch net-next-2.6 7/8] net: introduce rx_handler results and logic around that
From: Changli Gao @ 2011-03-05 16:45 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: netdev, davem, shemminger, kaber, fubar, eric.dumazet,
	nicolas.2p.debian, andy
In-Reply-To: <1299313794-5218-8-git-send-email-jpirko@redhat.com>

On Sat, Mar 5, 2011 at 4:29 PM, Jiri Pirko <jpirko@redhat.com> wrote:
>
> -typedef struct sk_buff *rx_handler_func_t(struct sk_buff *skb);
> +enum rx_handler_result {
> +       RX_HANDLER_CONSUMED,
> +       RX_HANDLER_ANOTHER,
> +       RX_HANDLER_EXACT,
> +       RX_HANDLER_PASS,
> +};

Why not extend the macros NET_RX_SUCCESS and NET_RX_DROP?  Then the
callers of __netif_receive_skb() may get the right congestion info
from its return value.

-- 
Regards,
Changli Gao(xiaosuo@gmail.com)

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox