netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net] bonding: set correct vlan id for alb xmit path
@ 2014-03-11  1:57 Ding Tianhong
  2014-03-11  2:15 ` Ding Tianhong
  2014-03-12  9:31 ` Ding Tianhong
  0 siblings, 2 replies; 5+ messages in thread
From: Ding Tianhong @ 2014-03-11  1:57 UTC (permalink / raw)
  To: Jay Vosburgh, Veaceslav Falico, Andy Gospodarek, David S. Miller, Netdev

The commit d3ab3ffd1d728d7ee77340e7e7e2c7cfe6a4013e
(bonding: use rlb_client_info->vlan_id instead of ->tag)
remove the rlb_client_info->tag, but occur some issues,
The vlan_get_tag() will return 0 for success and -EINVAL for
error, so the client_info->vlan_id always be set to 0 if the
vlan_get_tag return 0 for success, so the client_info would
never get a correct vlan id, and could not send a skb for vlan id 0.

Fix this by convert the client_info->vlan_id from u16 to s16,
and set to -1 for default value, the s16 is enough for the range
of the vlan id.

Fixes: d3ab3ffd1d7 (bonding: use rlb_client_info->vlan_id instead of ->tag)

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/bond_alb.c | 8 ++++----
 drivers/net/bonding/bond_alb.h | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index aaeeacf..fa95f70 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -539,7 +539,7 @@ static void rlb_update_client(struct rlb_client_info *client_info)
 
 		skb->dev = client_info->slave->dev;
 
-		if (client_info->vlan_id) {
+		if (client_info->vlan_id >= 0) {
 			skb = vlan_put_tag(skb, htons(ETH_P_8021Q), client_info->vlan_id);
 			if (!skb) {
 				pr_err("%s: Error: failed to insert VLAN tag\n",
@@ -722,8 +722,8 @@ static struct slave *rlb_choose_channel(struct sk_buff *skb, struct bonding *bon
 			client_info->ntt = 0;
 		}
 
-		if (!vlan_get_tag(skb, &client_info->vlan_id))
-			client_info->vlan_id = 0;
+		if (vlan_get_tag(skb, &client_info->vlan_id))
+			client_info->vlan_id = -1;
 
 		if (!client_info->assigned) {
 			u32 prev_tbl_head = bond_info->rx_hashtbl_used_head;
@@ -827,7 +827,7 @@ static void rlb_init_table_entry_dst(struct rlb_client_info *entry)
 	entry->used_prev = RLB_NULL_INDEX;
 	entry->assigned = 0;
 	entry->slave = NULL;
-	entry->vlan_id = 0;
+	entry->vlan_id = -1;
 }
 static void rlb_init_table_entry_src(struct rlb_client_info *entry)
 {
diff --git a/drivers/net/bonding/bond_alb.h b/drivers/net/bonding/bond_alb.h
index e09dd4bf..b49af98 100644
--- a/drivers/net/bonding/bond_alb.h
+++ b/drivers/net/bonding/bond_alb.h
@@ -125,7 +125,7 @@ struct rlb_client_info {
 	u8  assigned;		/* checking whether this entry is assigned */
 	u8  ntt;		/* flag - need to transmit client info */
 	struct slave *slave;	/* the slave assigned to this client */
-	unsigned short vlan_id;	/* VLAN tag associated with IP address */
+	short vlan_id;		/* VLAN tag associated with IP address */
 };
 
 struct tlb_slave_info {
-- 
1.8.0

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH net] bonding: set correct vlan id for alb xmit path
  2014-03-11  1:57 [PATCH net] bonding: set correct vlan id for alb xmit path Ding Tianhong
@ 2014-03-11  2:15 ` Ding Tianhong
  2014-03-12  9:31 ` Ding Tianhong
  1 sibling, 0 replies; 5+ messages in thread
From: Ding Tianhong @ 2014-03-11  2:15 UTC (permalink / raw)
  To: Jay Vosburgh, Veaceslav Falico, Andy Gospodarek, David S. Miller, Netdev

On 2014/3/11 9:57, Ding Tianhong wrote:
> The commit d3ab3ffd1d728d7ee77340e7e7e2c7cfe6a4013e
> (bonding: use rlb_client_info->vlan_id instead of ->tag)
> remove the rlb_client_info->tag, but occur some issues,
> The vlan_get_tag() will return 0 for success and -EINVAL for
> error, so the client_info->vlan_id always be set to 0 if the
> vlan_get_tag return 0 for success, so the client_info would
> never get a correct vlan id, and could not send a skb for vlan id 0.
> 
> Fix this by convert the client_info->vlan_id from u16 to s16,
> and set to -1 for default value, the s16 is enough for the range
> of the vlan id.
> 
> Fixes: d3ab3ffd1d7 (bonding: use rlb_client_info->vlan_id instead of ->tag)
> 
> 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>

Wrong CC list, sorry for that, resend.

> ---
>  drivers/net/bonding/bond_alb.c | 8 ++++----
>  drivers/net/bonding/bond_alb.h | 2 +-
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
> index aaeeacf..fa95f70 100644
> --- a/drivers/net/bonding/bond_alb.c
> +++ b/drivers/net/bonding/bond_alb.c
> @@ -539,7 +539,7 @@ static void rlb_update_client(struct rlb_client_info *client_info)
>  
>  		skb->dev = client_info->slave->dev;
>  
> -		if (client_info->vlan_id) {
> +		if (client_info->vlan_id >= 0) {
>  			skb = vlan_put_tag(skb, htons(ETH_P_8021Q), client_info->vlan_id);
>  			if (!skb) {
>  				pr_err("%s: Error: failed to insert VLAN tag\n",
> @@ -722,8 +722,8 @@ static struct slave *rlb_choose_channel(struct sk_buff *skb, struct bonding *bon
>  			client_info->ntt = 0;
>  		}
>  
> -		if (!vlan_get_tag(skb, &client_info->vlan_id))
> -			client_info->vlan_id = 0;
> +		if (vlan_get_tag(skb, &client_info->vlan_id))
> +			client_info->vlan_id = -1;
>  
>  		if (!client_info->assigned) {
>  			u32 prev_tbl_head = bond_info->rx_hashtbl_used_head;
> @@ -827,7 +827,7 @@ static void rlb_init_table_entry_dst(struct rlb_client_info *entry)
>  	entry->used_prev = RLB_NULL_INDEX;
>  	entry->assigned = 0;
>  	entry->slave = NULL;
> -	entry->vlan_id = 0;
> +	entry->vlan_id = -1;
>  }
>  static void rlb_init_table_entry_src(struct rlb_client_info *entry)
>  {
> diff --git a/drivers/net/bonding/bond_alb.h b/drivers/net/bonding/bond_alb.h
> index e09dd4bf..b49af98 100644
> --- a/drivers/net/bonding/bond_alb.h
> +++ b/drivers/net/bonding/bond_alb.h
> @@ -125,7 +125,7 @@ struct rlb_client_info {
>  	u8  assigned;		/* checking whether this entry is assigned */
>  	u8  ntt;		/* flag - need to transmit client info */
>  	struct slave *slave;	/* the slave assigned to this client */
> -	unsigned short vlan_id;	/* VLAN tag associated with IP address */
> +	short vlan_id;		/* VLAN tag associated with IP address */
>  };
>  
>  struct tlb_slave_info {
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread
* [PATCH net] bonding: set correct vlan id for alb xmit path
  2014-03-11  1:57 [PATCH net] bonding: set correct vlan id for alb xmit path Ding Tianhong
  2014-03-11  2:15 ` Ding Tianhong
@ 2014-03-12  9:31 ` Ding Tianhong
  2014-03-12 10:37   ` Veaceslav Falico
  2014-03-13 19:45   ` David Miller
  1 sibling, 2 replies; 5+ messages in thread
From: Ding Tianhong @ 2014-03-12  9:31 UTC (permalink / raw)
  To: Jay Vosburgh, Veaceslav Falico, Andy Gospodarek, David S. Miller, Netdev

The commit d3ab3ffd1d728d7ee77340e7e7e2c7cfe6a4013e
(bonding: use rlb_client_info->vlan_id instead of ->tag)
remove the rlb_client_info->tag, but occur some issues,
The vlan_get_tag() will return 0 for success and -EINVAL for
error, so the client_info->vlan_id always be set to 0 if the
vlan_get_tag return 0 for success, so the client_info would
never get a correct vlan id.

We should only set the vlan id to 0 when the vlan_get_tag return error.

Fixes: d3ab3ffd1d7 (bonding: use rlb_client_info->vlan_id instead of ->tag)

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/bond_alb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index a2c4747..e8f133e 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -730,7 +730,7 @@ static struct slave *rlb_choose_channel(struct sk_buff *skb, struct bonding *bon
 			client_info->ntt = 0;
 		}
 
-		if (!vlan_get_tag(skb, &client_info->vlan_id))
+		if (vlan_get_tag(skb, &client_info->vlan_id))
 			client_info->vlan_id = 0;
 
 		if (!client_info->assigned) {
-- 
1.8.0

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH net] bonding: set correct vlan id for alb xmit path
  2014-03-12  9:31 ` Ding Tianhong
@ 2014-03-12 10:37   ` Veaceslav Falico
  2014-03-13 19:45   ` David Miller
  1 sibling, 0 replies; 5+ messages in thread
From: Veaceslav Falico @ 2014-03-12 10:37 UTC (permalink / raw)
  To: Ding Tianhong; +Cc: Jay Vosburgh, Andy Gospodarek, David S. Miller, Netdev

On Wed, Mar 12, 2014 at 05:31:59PM +0800, Ding Tianhong wrote:
>The commit d3ab3ffd1d728d7ee77340e7e7e2c7cfe6a4013e
>(bonding: use rlb_client_info->vlan_id instead of ->tag)
>remove the rlb_client_info->tag, but occur some issues,
>The vlan_get_tag() will return 0 for success and -EINVAL for
>error, so the client_info->vlan_id always be set to 0 if the
>vlan_get_tag return 0 for success, so the client_info would
>never get a correct vlan id.
>
>We should only set the vlan id to 0 when the vlan_get_tag return error.
>
>Fixes: d3ab3ffd1d7 (bonding: use rlb_client_info->vlan_id instead of ->tag)
>
>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>

Acked-by: Veaceslav Falico <vfalico@redhat.com>

>---
> drivers/net/bonding/bond_alb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
>index a2c4747..e8f133e 100644
>--- a/drivers/net/bonding/bond_alb.c
>+++ b/drivers/net/bonding/bond_alb.c
>@@ -730,7 +730,7 @@ static struct slave *rlb_choose_channel(struct sk_buff *skb, struct bonding *bon
> 			client_info->ntt = 0;
> 		}
>
>-		if (!vlan_get_tag(skb, &client_info->vlan_id))
>+		if (vlan_get_tag(skb, &client_info->vlan_id))
> 			client_info->vlan_id = 0;
>
> 		if (!client_info->assigned) {
>-- 
>1.8.0
>
>
>

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH net] bonding: set correct vlan id for alb xmit path
  2014-03-12  9:31 ` Ding Tianhong
  2014-03-12 10:37   ` Veaceslav Falico
@ 2014-03-13 19:45   ` David Miller
  1 sibling, 0 replies; 5+ messages in thread
From: David Miller @ 2014-03-13 19:45 UTC (permalink / raw)
  To: dingtianhong; +Cc: fubar, vfalico, andy, netdev

From: Ding Tianhong <dingtianhong@huawei.com>
Date: Wed, 12 Mar 2014 17:31:59 +0800

> The commit d3ab3ffd1d728d7ee77340e7e7e2c7cfe6a4013e
> (bonding: use rlb_client_info->vlan_id instead of ->tag)
> remove the rlb_client_info->tag, but occur some issues,
> The vlan_get_tag() will return 0 for success and -EINVAL for
> error, so the client_info->vlan_id always be set to 0 if the
> vlan_get_tag return 0 for success, so the client_info would
> never get a correct vlan id.
> 
> We should only set the vlan id to 0 when the vlan_get_tag return error.
> 
> Fixes: d3ab3ffd1d7 (bonding: use rlb_client_info->vlan_id instead of ->tag)
> 
> 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>

Applied, thanks.

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-03-13 19:45 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-03-11  1:57 [PATCH net] bonding: set correct vlan id for alb xmit path Ding Tianhong
2014-03-11  2:15 ` Ding Tianhong
2014-03-12  9:31 ` Ding Tianhong
2014-03-12 10:37   ` Veaceslav Falico
2014-03-13 19:45   ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).