[PATCH net-next 0/3] bonding: slight optimization and cleanup

[PATCH net-next 0/3] bonding: slight optimization and cleanup

[Ding Tianhong]

Ding Tianhong (3):
  bonding: remove the unnecessary struct bond_net
  bonding: simplify the slave_do_arp_validate_only()
  bonding: remove the unused macro

 drivers/net/bonding/bond_main.c |  4 ++--
 drivers/net/bonding/bonding.h   | 23 +++++++++--------------
 2 files changed, 11 insertions(+), 16 deletions(-)

-- 
1.8.0

[PATCH net-next 1/3] bonding: remove the unnecessary struct bond_net

[Ding Tianhong]

Move the structure bond_net forward, and remove the unnecessary structure declaration.

Cc: Ding Tianhong <dingtianhong@huawei.com>
Cc: Jay Vosburgh <fubar@us.ibm.com>
Cc: Andy Gospodarek <andy@greyhouse.net>
Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
 drivers/net/bonding/bonding.h | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index b8bdd0a..9f23bf3 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -487,7 +487,14 @@ static inline bool slave_can_tx(struct slave *slave)
 		return false;
 }
 
-struct bond_net;
+struct bond_net {
+	struct net *		net;	/* Associated network namespace */
+	struct list_head	dev_list;
+#ifdef CONFIG_PROC_FS
+	struct proc_dir_entry *	proc_dir;
+#endif
+	struct class_attribute	class_attr_bonding_masters;
+};
 
 int bond_arp_rcv(const struct sk_buff *skb, struct bonding *bond, struct slave *slave);
 void bond_dev_queue_xmit(struct bonding *bond, struct sk_buff *skb, struct net_device *slave_dev);
@@ -516,15 +523,6 @@ struct net_device *bond_option_active_slave_get_rcu(struct bonding *bond);
 struct net_device *bond_option_active_slave_get(struct bonding *bond);
 const char *bond_slave_link_status(s8 link);
 
-struct bond_net {
-	struct net *		net;	/* Associated network namespace */
-	struct list_head	dev_list;
-#ifdef CONFIG_PROC_FS
-	struct proc_dir_entry *	proc_dir;
-#endif
-	struct class_attribute	class_attr_bonding_masters;
-};
-
 #ifdef CONFIG_PROC_FS
 void bond_create_proc_entry(struct bonding *bond);
 void bond_remove_proc_entry(struct bonding *bond);
-- 
1.8.0

[PATCH net-next 2/3] bonding: simplify the slave_do_arp_validate_only()

[Ding Tianhong]

The argument slave is not used for slave_do_arp_validate_only(), so no need
to keep it, make the function more simple.

Cc: Jay Vosburgh <fubar@us.ibm.com>
Cc: Veaceslav Falico <vfalico@redhat.com>
Cc: Andy Gospodarek <andy@greyhouse.net>
Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
 drivers/net/bonding/bond_main.c | 4 ++--
 drivers/net/bonding/bonding.h   | 3 +--
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 69aff72..ff73c14 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2291,8 +2291,8 @@ int bond_arp_rcv(const struct sk_buff *skb, struct bonding *bond,
 	int alen, is_arp = skb->protocol == __cpu_to_be16(ETH_P_ARP);
 
 	if (!slave_do_arp_validate(bond, slave)) {
-		if ((slave_do_arp_validate_only(bond, slave) && is_arp) ||
-		    !slave_do_arp_validate_only(bond, slave))
+		if ((slave_do_arp_validate_only(bond) && is_arp) ||
+		    !slave_do_arp_validate_only(bond))
 			slave->last_rx = jiffies;
 		return RX_HANDLER_ANOTHER;
 	} else if (!is_arp) {
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 9f23bf3..5d1e1a8 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -395,8 +395,7 @@ static inline int slave_do_arp_validate(struct bonding *bond,
 	return bond->params.arp_validate & (1 << bond_slave_state(slave));
 }
 
-static inline int slave_do_arp_validate_only(struct bonding *bond,
-					     struct slave *slave)
+static inline int slave_do_arp_validate_only(struct bonding *bond)
 {
 	return bond->params.arp_validate & BOND_ARP_FILTER;
 }
-- 
1.8.0

[PATCH net-next 3/3] bonding: remove the unused macro

[Ding Tianhong]

Cc: Jay Vosburgh <fubar@us.ibm.com>
Cc: Veaceslav Falico <vfalico@redhat.com>
Cc: Andy Gospodarek <andy@greyhouse.net>
Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
 drivers/net/bonding/bonding.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 5d1e1a8..f462f6f 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -182,8 +182,6 @@ struct bond_parm_tbl {
 	int mode;
 };
 
-#define BOND_MAX_MODENAME_LEN 20
-
 struct slave {
 	struct net_device *dev; /* first - useful for panic debug */
 	struct bonding *bond; /* our master */
-- 
1.8.0

Re: [PATCH net-next 1/3] bonding: remove the unnecessary struct bond_net

[Veaceslav Falico]

On Wed, May 07, 2014 at 11:45:17AM +0800, Ding Tianhong wrote:
>Move the structure bond_net forward, and remove the unnecessary structure declaration.
>
>Cc: Ding Tianhong <dingtianhong@huawei.com>
>Cc: Jay Vosburgh <fubar@us.ibm.com>
>Cc: Andy Gospodarek <andy@greyhouse.net>
>Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
>---
> drivers/net/bonding/bonding.h | 18 ++++++++----------
> 1 file changed, 8 insertions(+), 10 deletions(-)
>
>diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
>index b8bdd0a..9f23bf3 100644
>--- a/drivers/net/bonding/bonding.h
>+++ b/drivers/net/bonding/bonding.h
>@@ -487,7 +487,14 @@ static inline bool slave_can_tx(struct slave *slave)
> 		return false;
> }
>
>-struct bond_net;
>+struct bond_net {
>+	struct net *		net;	/* Associated network namespace */

You might as well clean this up to look like "foo *bar;".

>+	struct list_head	dev_list;
>+#ifdef CONFIG_PROC_FS
>+	struct proc_dir_entry *	proc_dir;

Ditto.

>+#endif
>+	struct class_attribute	class_attr_bonding_masters;
>+};
>
> int bond_arp_rcv(const struct sk_buff *skb, struct bonding *bond, struct slave *slave);
> void bond_dev_queue_xmit(struct bonding *bond, struct sk_buff *skb, struct net_device *slave_dev);
>@@ -516,15 +523,6 @@ struct net_device *bond_option_active_slave_get_rcu(struct bonding *bond);
> struct net_device *bond_option_active_slave_get(struct bonding *bond);
> const char *bond_slave_link_status(s8 link);
>
>-struct bond_net {
>-	struct net *		net;	/* Associated network namespace */
>-	struct list_head	dev_list;
>-#ifdef CONFIG_PROC_FS
>-	struct proc_dir_entry *	proc_dir;
>-#endif
>-	struct class_attribute	class_attr_bonding_masters;
>-};
>-
> #ifdef CONFIG_PROC_FS
> void bond_create_proc_entry(struct bonding *bond);
> void bond_remove_proc_entry(struct bonding *bond);
>-- 
>1.8.0
>
>

Re: [PATCH net-next 0/3] bonding: slight optimization and cleanup

[Veaceslav Falico]

On Wed, May 07, 2014 at 11:45:16AM +0800, Ding Tianhong wrote:
>Ding Tianhong (3):
>  bonding: remove the unnecessary struct bond_net
>  bonding: simplify the slave_do_arp_validate_only()
>  bonding: remove the unused macro


Except that remark about first patch - looks good to me.

>
> drivers/net/bonding/bond_main.c |  4 ++--
> drivers/net/bonding/bonding.h   | 23 +++++++++--------------
> 2 files changed, 11 insertions(+), 16 deletions(-)
>
>-- 
>1.8.0
>
>

Re: [PATCH net-next 0/3] bonding: slight optimization and cleanup

[Veaceslav Falico]

On Wed, May 07, 2014 at 03:27:58PM +0200, Veaceslav Falico wrote:
>On Wed, May 07, 2014 at 11:45:16AM +0800, Ding Tianhong wrote:
>>Ding Tianhong (3):
>> bonding: remove the unnecessary struct bond_net
>> bonding: simplify the slave_do_arp_validate_only()
>> bonding: remove the unused macro
>
>
>Except that remark about first patch - looks good to me.

Also, please update Maintainer's email addresses, get them from MAINTAINERS
file. Jay's bounces, mine still not :) but I'm filtering them out.

>
>>
>>drivers/net/bonding/bond_main.c |  4 ++--
>>drivers/net/bonding/bonding.h   | 23 +++++++++--------------
>>2 files changed, 11 insertions(+), 16 deletions(-)
>>
>>-- 
>>1.8.0
>>
>>

Re: [PATCH net-next 1/3] bonding: remove the unnecessary struct bond_net

[Ding Tianhong]

On 2014/5/7 21:26, Veaceslav Falico wrote:
> On Wed, May 07, 2014 at 11:45:17AM +0800, Ding Tianhong wrote:
>> Move the structure bond_net forward, and remove the unnecessary structure declaration.
>>
>> Cc: Ding Tianhong <dingtianhong@huawei.com>
>> Cc: Jay Vosburgh <fubar@us.ibm.com>
>> Cc: Andy Gospodarek <andy@greyhouse.net>
>> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
>> ---
>> drivers/net/bonding/bonding.h | 18 ++++++++----------
>> 1 file changed, 8 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
>> index b8bdd0a..9f23bf3 100644
>> --- a/drivers/net/bonding/bonding.h
>> +++ b/drivers/net/bonding/bonding.h
>> @@ -487,7 +487,14 @@ static inline bool slave_can_tx(struct slave *slave)
>>         return false;
>> }
>>
>> -struct bond_net;
>> +struct bond_net {
>> +    struct net *        net;    /* Associated network namespace */
> 
> You might as well clean this up to look like "foo *bar;".
> 

Yes, thanks for your piercing eyes, I need to be more careful.

Ding

>> +    struct list_head    dev_list;
>> +#ifdef CONFIG_PROC_FS
>> +    struct proc_dir_entry *    proc_dir;
> 
> Ditto.
> 
>> +#endif
>> +    struct class_attribute    class_attr_bonding_masters;
>> +};
>>
>> int bond_arp_rcv(const struct sk_buff *skb, struct bonding *bond, struct slave *slave);
>> void bond_dev_queue_xmit(struct bonding *bond, struct sk_buff *skb, struct net_device *slave_dev);
>> @@ -516,15 +523,6 @@ struct net_device *bond_option_active_slave_get_rcu(struct bonding *bond);
>> struct net_device *bond_option_active_slave_get(struct bonding *bond);
>> const char *bond_slave_link_status(s8 link);
>>
>> -struct bond_net {
>> -    struct net *        net;    /* Associated network namespace */
>> -    struct list_head    dev_list;
>> -#ifdef CONFIG_PROC_FS
>> -    struct proc_dir_entry *    proc_dir;
>> -#endif
>> -    struct class_attribute    class_attr_bonding_masters;
>> -};
>> -
>> #ifdef CONFIG_PROC_FS
>> void bond_create_proc_entry(struct bonding *bond);
>> void bond_remove_proc_entry(struct bonding *bond);
>> -- 
>> 1.8.0
>>
>>
> 
> .
>