* [PATCH] fcoe: correct checking for bonding @ 2011-02-28 13:32 Jiri Pirko 2011-02-28 17:15 ` Jay Vosburgh 0 siblings, 1 reply; 9+ messages in thread From: Jiri Pirko @ 2011-02-28 13:32 UTC (permalink / raw) To: linux-scsi; +Cc: devel, robert.w.love, James.Bottomley, netdev Check for IFF_BONDING as this flag is set-up for all bonding devices. Signed-off-by: Jiri Pirko <jpirko@redhat.com> --- drivers/scsi/fcoe/fcoe.c | 4 +--- 1 files changed, 1 insertions(+), 3 deletions(-) diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c index 9f9600b..67714a4 100644 --- a/drivers/scsi/fcoe/fcoe.c +++ b/drivers/scsi/fcoe/fcoe.c @@ -285,9 +285,7 @@ static int fcoe_interface_setup(struct fcoe_interface *fcoe, } /* Do not support for bonding device */ - if ((netdev->priv_flags & IFF_MASTER_ALB) || - (netdev->priv_flags & IFF_SLAVE_INACTIVE) || - (netdev->priv_flags & IFF_MASTER_8023AD)) { + if (netdev->priv_flags & IFF_BONDING) { FCOE_NETDEV_DBG(netdev, "Bonded interfaces not supported\n"); return -EOPNOTSUPP; } -- 1.7.3.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] fcoe: correct checking for bonding 2011-02-28 13:32 [PATCH] fcoe: correct checking for bonding Jiri Pirko @ 2011-02-28 17:15 ` Jay Vosburgh 2011-02-28 17:54 ` [Open-FCoE] " Joe Eykholt 0 siblings, 1 reply; 9+ messages in thread From: Jay Vosburgh @ 2011-02-28 17:15 UTC (permalink / raw) To: Jiri Pirko; +Cc: linux-scsi, devel, robert.w.love, James.Bottomley, netdev Jiri Pirko <jpirko@redhat.com> wrote: >Check for IFF_BONDING as this flag is set-up for all bonding devices. > >Signed-off-by: Jiri Pirko <jpirko@redhat.com> >--- > drivers/scsi/fcoe/fcoe.c | 4 +--- > 1 files changed, 1 insertions(+), 3 deletions(-) > >diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c >index 9f9600b..67714a4 100644 >--- a/drivers/scsi/fcoe/fcoe.c >+++ b/drivers/scsi/fcoe/fcoe.c >@@ -285,9 +285,7 @@ static int fcoe_interface_setup(struct fcoe_interface *fcoe, > } > > /* Do not support for bonding device */ >- if ((netdev->priv_flags & IFF_MASTER_ALB) || >- (netdev->priv_flags & IFF_SLAVE_INACTIVE) || >- (netdev->priv_flags & IFF_MASTER_8023AD)) { >+ if (netdev->priv_flags & IFF_BONDING) { > FCOE_NETDEV_DBG(netdev, "Bonded interfaces not supported\n"); > return -EOPNOTSUPP; > } Based on past discussions, I believe the intent of the code is to permit FCOE over bonding only for active-backup mode, and possibly for -xor/-rr as well. I'm not sure if the slave or the master is what's being tested here, so I'm not sure what the right thing to do is. I suspect it's the master, as I recall discussion of one configuration involving active-backup mode balancing FCOE traffic over both the active and inactive slaves. FCOE uses the "orig_dev" logic in __netif_receive_skb to have the packets delivered even on the nominally inactive slave. -J --- -Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Open-FCoE] [PATCH] fcoe: correct checking for bonding 2011-02-28 17:15 ` Jay Vosburgh @ 2011-02-28 17:54 ` Joe Eykholt 2011-03-01 6:37 ` Jiri Pirko 0 siblings, 1 reply; 9+ messages in thread From: Joe Eykholt @ 2011-02-28 17:54 UTC (permalink / raw) To: Jay Vosburgh; +Cc: Jiri Pirko, James.Bottomley, netdev, devel, linux-scsi On 2/28/11 9:15 AM, Jay Vosburgh wrote: > Jiri Pirko<jpirko@redhat.com> wrote: > >> Check for IFF_BONDING as this flag is set-up for all bonding devices. >> >> Signed-off-by: Jiri Pirko<jpirko@redhat.com> >> --- >> drivers/scsi/fcoe/fcoe.c | 4 +--- >> 1 files changed, 1 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c >> index 9f9600b..67714a4 100644 >> --- a/drivers/scsi/fcoe/fcoe.c >> +++ b/drivers/scsi/fcoe/fcoe.c >> @@ -285,9 +285,7 @@ static int fcoe_interface_setup(struct fcoe_interface *fcoe, >> } >> >> /* Do not support for bonding device */ >> - if ((netdev->priv_flags& IFF_MASTER_ALB) || >> - (netdev->priv_flags& IFF_SLAVE_INACTIVE) || >> - (netdev->priv_flags& IFF_MASTER_8023AD)) { >> + if (netdev->priv_flags& IFF_BONDING) { >> FCOE_NETDEV_DBG(netdev, "Bonded interfaces not supported\n"); >> return -EOPNOTSUPP; >> } > > Based on past discussions, I believe the intent of the code is > to permit FCOE over bonding only for active-backup mode, and possibly > for -xor/-rr as well. > > I'm not sure if the slave or the master is what's being tested > here, so I'm not sure what the right thing to do is. I suspect it's the > master, as I recall discussion of one configuration involving > active-backup mode balancing FCOE traffic over both the active and > inactive slaves. FCOE uses the "orig_dev" logic in __netif_receive_skb > to have the packets delivered even on the nominally inactive slave. > > -J > > --- > -Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com Right. That was the intent. It should work on the physical dev, but probably not on the master of the bond. If you have a master/slave bond for IPv4 between eth1 and eth2, say, and they are going to two different DCE (FCoE) switches, presumably on different VSANs but with ultimate access to the same disks, then you want to split the FCoE traffic in active/active mode using separate FCoE instances on eth1 and eth2 even though IP is using active/standby on bond0. This should work. But, putting fcoe on bond0 isn't going to do what you want. However, it seems like the check above shouldn't be checking IFF_SLAVE_INACTIVE. I can't test this. Joe ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Open-FCoE] [PATCH] fcoe: correct checking for bonding 2011-02-28 17:54 ` [Open-FCoE] " Joe Eykholt @ 2011-03-01 6:37 ` Jiri Pirko 2011-03-01 17:49 ` Joe Eykholt 0 siblings, 1 reply; 9+ messages in thread From: Jiri Pirko @ 2011-03-01 6:37 UTC (permalink / raw) To: Joe Eykholt; +Cc: Jay Vosburgh, James.Bottomley, netdev, devel, linux-scsi Mon, Feb 28, 2011 at 06:54:29PM CET, joe.eykholt@gmail.com wrote: >On 2/28/11 9:15 AM, Jay Vosburgh wrote: >>Jiri Pirko<jpirko@redhat.com> wrote: >> >>>Check for IFF_BONDING as this flag is set-up for all bonding devices. >>> >>>Signed-off-by: Jiri Pirko<jpirko@redhat.com> >>>--- >>>drivers/scsi/fcoe/fcoe.c | 4 +--- >>>1 files changed, 1 insertions(+), 3 deletions(-) >>> >>>diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c >>>index 9f9600b..67714a4 100644 >>>--- a/drivers/scsi/fcoe/fcoe.c >>>+++ b/drivers/scsi/fcoe/fcoe.c >>>@@ -285,9 +285,7 @@ static int fcoe_interface_setup(struct fcoe_interface *fcoe, >>> } >>> >>> /* Do not support for bonding device */ >>>- if ((netdev->priv_flags& IFF_MASTER_ALB) || >>>- (netdev->priv_flags& IFF_SLAVE_INACTIVE) || >>>- (netdev->priv_flags& IFF_MASTER_8023AD)) { >>>+ if (netdev->priv_flags& IFF_BONDING) { >>> FCOE_NETDEV_DBG(netdev, "Bonded interfaces not supported\n"); >>> return -EOPNOTSUPP; >>> } >> >> Based on past discussions, I believe the intent of the code is >>to permit FCOE over bonding only for active-backup mode, and possibly >>for -xor/-rr as well. >> >> I'm not sure if the slave or the master is what's being tested >>here, so I'm not sure what the right thing to do is. I suspect it's the >>master, as I recall discussion of one configuration involving >>active-backup mode balancing FCOE traffic over both the active and >>inactive slaves. FCOE uses the "orig_dev" logic in __netif_receive_skb >>to have the packets delivered even on the nominally inactive slave. >> >> -J >> >>--- >> -Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com > >Right. That was the intent. It should work on the physical dev, but probably >not on the master of the bond. > >If you have a master/slave bond for IPv4 between eth1 and eth2, say, >and they are going to two different DCE (FCoE) switches, presumably on >different VSANs but with ultimate access to the same disks, >then you want to split the FCoE traffic in active/active >mode using separate FCoE instances on eth1 and eth2 even though IP >is using active/standby on bond0. This should work. But, putting fcoe >on bond0 isn't going to do what you want. > >However, it seems like the check above shouldn't be checking >IFF_SLAVE_INACTIVE. I can't test this. OK. So I guess the right check should be for: (netdev->priv_flags & IFF_BONDING && netdev->flags & IFF_MASTER) This would disable adding all bond devices (like bond0 etc) and allows to use enslaved physdevs. Note that checking for mode is irrelevant here. Mode could be easily changed later without fcoe knowing that. Jirka > > Joe > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Open-FCoE] [PATCH] fcoe: correct checking for bonding 2011-03-01 6:37 ` Jiri Pirko @ 2011-03-01 17:49 ` Joe Eykholt 2011-03-01 18:01 ` Vasu Dev 2011-03-01 18:06 ` Jay Vosburgh 0 siblings, 2 replies; 9+ messages in thread From: Joe Eykholt @ 2011-03-01 17:49 UTC (permalink / raw) To: Jiri Pirko; +Cc: Jay Vosburgh, James.Bottomley, netdev, devel, linux-scsi On 2/28/11 10:37 PM, Jiri Pirko wrote: > Mon, Feb 28, 2011 at 06:54:29PM CET, joe.eykholt@gmail.com wrote: >> On 2/28/11 9:15 AM, Jay Vosburgh wrote: >>> Jiri Pirko<jpirko@redhat.com> wrote: >>> >>>> Check for IFF_BONDING as this flag is set-up for all bonding devices. >>>> >>>> Signed-off-by: Jiri Pirko<jpirko@redhat.com> >>>> --- >>>> drivers/scsi/fcoe/fcoe.c | 4 +--- >>>> 1 files changed, 1 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c >>>> index 9f9600b..67714a4 100644 >>>> --- a/drivers/scsi/fcoe/fcoe.c >>>> +++ b/drivers/scsi/fcoe/fcoe.c >>>> @@ -285,9 +285,7 @@ static int fcoe_interface_setup(struct fcoe_interface *fcoe, >>>> } >>>> >>>> /* Do not support for bonding device */ >>>> - if ((netdev->priv_flags& IFF_MASTER_ALB) || >>>> - (netdev->priv_flags& IFF_SLAVE_INACTIVE) || >>>> - (netdev->priv_flags& IFF_MASTER_8023AD)) { >>>> + if (netdev->priv_flags& IFF_BONDING) { >>>> FCOE_NETDEV_DBG(netdev, "Bonded interfaces not supported\n"); >>>> return -EOPNOTSUPP; >>>> } >>> >>> Based on past discussions, I believe the intent of the code is >>> to permit FCOE over bonding only for active-backup mode, and possibly >>> for -xor/-rr as well. >>> >>> I'm not sure if the slave or the master is what's being tested >>> here, so I'm not sure what the right thing to do is. I suspect it's the >>> master, as I recall discussion of one configuration involving >>> active-backup mode balancing FCOE traffic over both the active and >>> inactive slaves. FCOE uses the "orig_dev" logic in __netif_receive_skb >>> to have the packets delivered even on the nominally inactive slave. >>> >>> -J >>> >>> --- >>> -Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com >> >> Right. That was the intent. It should work on the physical dev, but probably >> not on the master of the bond. >> >> If you have a master/slave bond for IPv4 between eth1 and eth2, say, >> and they are going to two different DCE (FCoE) switches, presumably on >> different VSANs but with ultimate access to the same disks, >> then you want to split the FCoE traffic in active/active >> mode using separate FCoE instances on eth1 and eth2 even though IP >> is using active/standby on bond0. This should work. But, putting fcoe >> on bond0 isn't going to do what you want. >> >> However, it seems like the check above shouldn't be checking >> IFF_SLAVE_INACTIVE. I can't test this. > > OK. So I guess the right check should be for: > (netdev->priv_flags& IFF_BONDING&& netdev->flags& IFF_MASTER) I think that's OK. How about just checking for MASTER? When is MASTER going to be set without BONDING? Otherwise I'd add some parens or I might code this as: if ((netdev->priv_flags & (IFF_BONDING | IFF_MASTER)) == (IFF_BONDING | IFF_MASTER)) Which is less clear, I know, but used to generate better code. The compiler might generate the same code these days. Not that this is performance-critical or anything. > This would disable adding all bond devices (like bond0 etc) and allows > to use enslaved physdevs. > > Note that checking for mode is irrelevant here. Mode could be easily > changed later without fcoe knowing that. > > Jirka Cheers, Joe ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Open-FCoE] [PATCH] fcoe: correct checking for bonding 2011-03-01 17:49 ` Joe Eykholt @ 2011-03-01 18:01 ` Vasu Dev 2011-03-01 18:09 ` Vasu Dev 2011-03-01 18:06 ` Jay Vosburgh 1 sibling, 1 reply; 9+ messages in thread From: Vasu Dev @ 2011-03-01 18:01 UTC (permalink / raw) To: Joe Eykholt Cc: Jiri Pirko, James.Bottomley, netdev, Jay Vosburgh, devel, linux-scsi On Tue, 2011-03-01 at 09:49 -0800, Joe Eykholt wrote: > > Otherwise I'd add some parens or I might code this as: > > if ((netdev->priv_flags & (IFF_BONDING | IFF_MASTER)) == > (IFF_BONDING | IFF_MASTER)) or also skip == in code as:- if (netdev->priv_flags & (IFF_BONDING | IFF_MASTER)) Would least save few chars in source code. Vasu ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Open-FCoE] [PATCH] fcoe: correct checking for bonding 2011-03-01 18:01 ` Vasu Dev @ 2011-03-01 18:09 ` Vasu Dev 0 siblings, 0 replies; 9+ messages in thread From: Vasu Dev @ 2011-03-01 18:09 UTC (permalink / raw) To: Joe Eykholt Cc: linux-scsi, Jiri Pirko, netdev, Jay Vosburgh, James.Bottomley, devel On Tue, 2011-03-01 at 10:01 -0800, Vasu Dev wrote: > On Tue, 2011-03-01 at 09:49 -0800, Joe Eykholt wrote: > > > > Otherwise I'd add some parens or I might code this as: > > > > if ((netdev->priv_flags & (IFF_BONDING | IFF_MASTER)) == > > (IFF_BONDING | IFF_MASTER)) > > or also skip == in code as:- > > if (netdev->priv_flags & (IFF_BONDING | IFF_MASTER)) > > Would least save few chars in source code. Never mind, you wanted to check both bits set instead checking only one of them, my mistake, anycase these are in different fields as another most recent response mentioned. Vasu ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Open-FCoE] [PATCH] fcoe: correct checking for bonding 2011-03-01 17:49 ` Joe Eykholt 2011-03-01 18:01 ` Vasu Dev @ 2011-03-01 18:06 ` Jay Vosburgh 2011-03-01 20:12 ` Jiri Pirko 1 sibling, 1 reply; 9+ messages in thread From: Jay Vosburgh @ 2011-03-01 18:06 UTC (permalink / raw) To: Joe Eykholt; +Cc: Jiri Pirko, James.Bottomley, netdev, devel, linux-scsi Joe Eykholt <joe.eykholt@gmail.com> wrote: >On 2/28/11 10:37 PM, Jiri Pirko wrote: >> Mon, Feb 28, 2011 at 06:54:29PM CET, joe.eykholt@gmail.com wrote: >>> On 2/28/11 9:15 AM, Jay Vosburgh wrote: >>>> Jiri Pirko<jpirko@redhat.com> wrote: >>>> >>>>> Check for IFF_BONDING as this flag is set-up for all bonding devices. >>>>> >>>>> Signed-off-by: Jiri Pirko<jpirko@redhat.com> >>>>> --- >>>>> drivers/scsi/fcoe/fcoe.c | 4 +--- >>>>> 1 files changed, 1 insertions(+), 3 deletions(-) >>>>> >>>>> diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c >>>>> index 9f9600b..67714a4 100644 >>>>> --- a/drivers/scsi/fcoe/fcoe.c >>>>> +++ b/drivers/scsi/fcoe/fcoe.c >>>>> @@ -285,9 +285,7 @@ static int fcoe_interface_setup(struct fcoe_interface *fcoe, >>>>> } >>>>> >>>>> /* Do not support for bonding device */ >>>>> - if ((netdev->priv_flags& IFF_MASTER_ALB) || >>>>> - (netdev->priv_flags& IFF_SLAVE_INACTIVE) || >>>>> - (netdev->priv_flags& IFF_MASTER_8023AD)) { >>>>> + if (netdev->priv_flags& IFF_BONDING) { >>>>> FCOE_NETDEV_DBG(netdev, "Bonded interfaces not supported\n"); >>>>> return -EOPNOTSUPP; >>>>> } >>>> >>>> Based on past discussions, I believe the intent of the code is >>>> to permit FCOE over bonding only for active-backup mode, and possibly >>>> for -xor/-rr as well. >>>> >>>> I'm not sure if the slave or the master is what's being tested >>>> here, so I'm not sure what the right thing to do is. I suspect it's the >>>> master, as I recall discussion of one configuration involving >>>> active-backup mode balancing FCOE traffic over both the active and >>>> inactive slaves. FCOE uses the "orig_dev" logic in __netif_receive_skb >>>> to have the packets delivered even on the nominally inactive slave. >>>> >>>> -J >>>> >>>> --- >>>> -Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com >>> >>> Right. That was the intent. It should work on the physical dev, but probably >>> not on the master of the bond. >>> >>> If you have a master/slave bond for IPv4 between eth1 and eth2, say, >>> and they are going to two different DCE (FCoE) switches, presumably on >>> different VSANs but with ultimate access to the same disks, >>> then you want to split the FCoE traffic in active/active >>> mode using separate FCoE instances on eth1 and eth2 even though IP >>> is using active/standby on bond0. This should work. But, putting fcoe >>> on bond0 isn't going to do what you want. >>> >>> However, it seems like the check above shouldn't be checking >>> IFF_SLAVE_INACTIVE. I can't test this. >> >> OK. So I guess the right check should be for: >> (netdev->priv_flags& IFF_BONDING&& netdev->flags& IFF_MASTER) > >I think that's OK. How about just checking for MASTER? >When is MASTER going to be set without BONDING? One or two other things besides bonding use IFF_MASTER, but IFF_BONDING is only for bonding. >Otherwise I'd add some parens or I might code this as: > > if ((netdev->priv_flags & (IFF_BONDING | IFF_MASTER)) == > (IFF_BONDING | IFF_MASTER)) This doesn't work because the flags are kept in different places, IFF_MASTER is in flags and IFF_BONDING in priv_flags. -J >Which is less clear, I know, but used to generate better code. >The compiler might generate the same code these days. >Not that this is performance-critical or anything. > >> This would disable adding all bond devices (like bond0 etc) and allows >> to use enslaved physdevs. >> >> Note that checking for mode is irrelevant here. Mode could be easily >> changed later without fcoe knowing that. This is also true. -J --- -Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] fcoe: correct checking for bonding 2011-03-01 18:06 ` Jay Vosburgh @ 2011-03-01 20:12 ` Jiri Pirko 0 siblings, 0 replies; 9+ messages in thread From: Jiri Pirko @ 2011-03-01 20:12 UTC (permalink / raw) To: Jay Vosburgh Cc: James.Bottomley-d9PhHud1JfjCXq6kfMZ53/egYHeGw8Jk, netdev-u79uwXL29TY76Z2rM5mHXA, devel-s9riP+hp16TNLxjTenLetw, linux-scsi-u79uwXL29TY76Z2rM5mHXA Tue, Mar 01, 2011 at 07:06:02PM CET, fubar-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org wrote: >Joe Eykholt <joe.eykholt-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: > >>On 2/28/11 10:37 PM, Jiri Pirko wrote: >>> Mon, Feb 28, 2011 at 06:54:29PM CET, joe.eykholt-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote: >>>> On 2/28/11 9:15 AM, Jay Vosburgh wrote: >>>>> Jiri Pirko<jpirko-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote: >>>>> >>>>>> Check for IFF_BONDING as this flag is set-up for all bonding devices. >>>>>> >>>>>> Signed-off-by: Jiri Pirko<jpirko-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> >>>>>> --- >>>>>> drivers/scsi/fcoe/fcoe.c | 4 +--- >>>>>> 1 files changed, 1 insertions(+), 3 deletions(-) >>>>>> >>>>>> diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c >>>>>> index 9f9600b..67714a4 100644 >>>>>> --- a/drivers/scsi/fcoe/fcoe.c >>>>>> +++ b/drivers/scsi/fcoe/fcoe.c >>>>>> @@ -285,9 +285,7 @@ static int fcoe_interface_setup(struct fcoe_interface *fcoe, >>>>>> } >>>>>> >>>>>> /* Do not support for bonding device */ >>>>>> - if ((netdev->priv_flags& IFF_MASTER_ALB) || >>>>>> - (netdev->priv_flags& IFF_SLAVE_INACTIVE) || >>>>>> - (netdev->priv_flags& IFF_MASTER_8023AD)) { >>>>>> + if (netdev->priv_flags& IFF_BONDING) { >>>>>> FCOE_NETDEV_DBG(netdev, "Bonded interfaces not supported\n"); >>>>>> return -EOPNOTSUPP; >>>>>> } >>>>> >>>>> Based on past discussions, I believe the intent of the code is >>>>> to permit FCOE over bonding only for active-backup mode, and possibly >>>>> for -xor/-rr as well. >>>>> >>>>> I'm not sure if the slave or the master is what's being tested >>>>> here, so I'm not sure what the right thing to do is. I suspect it's the >>>>> master, as I recall discussion of one configuration involving >>>>> active-backup mode balancing FCOE traffic over both the active and >>>>> inactive slaves. FCOE uses the "orig_dev" logic in __netif_receive_skb >>>>> to have the packets delivered even on the nominally inactive slave. >>>>> >>>>> -J >>>>> >>>>> --- >>>>> -Jay Vosburgh, IBM Linux Technology Center, fubar-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org >>>> >>>> Right. That was the intent. It should work on the physical dev, but probably >>>> not on the master of the bond. >>>> >>>> If you have a master/slave bond for IPv4 between eth1 and eth2, say, >>>> and they are going to two different DCE (FCoE) switches, presumably on >>>> different VSANs but with ultimate access to the same disks, >>>> then you want to split the FCoE traffic in active/active >>>> mode using separate FCoE instances on eth1 and eth2 even though IP >>>> is using active/standby on bond0. This should work. But, putting fcoe >>>> on bond0 isn't going to do what you want. >>>> >>>> However, it seems like the check above shouldn't be checking >>>> IFF_SLAVE_INACTIVE. I can't test this. >>> >>> OK. So I guess the right check should be for: >>> (netdev->priv_flags& IFF_BONDING&& netdev->flags& IFF_MASTER) >> >>I think that's OK. How about just checking for MASTER? >>When is MASTER going to be set without BONDING? > > One or two other things besides bonding use IFF_MASTER, but >IFF_BONDING is only for bonding. > >>Otherwise I'd add some parens or I might code this as: >> >> if ((netdev->priv_flags & (IFF_BONDING | IFF_MASTER)) == >> (IFF_BONDING | IFF_MASTER)) > > This doesn't work because the flags are kept in different >places, IFF_MASTER is in flags and IFF_BONDING in priv_flags. Exactly - I'm going to send corrected patch very soon. Jirka > > -J > >>Which is less clear, I know, but used to generate better code. >>The compiler might generate the same code these days. >>Not that this is performance-critical or anything. >> >>> This would disable adding all bond devices (like bond0 etc) and allows >>> to use enslaved physdevs. >>> >>> Note that checking for mode is irrelevant here. Mode could be easily >>> changed later without fcoe knowing that. > > This is also true. > > -J > >--- > -Jay Vosburgh, IBM Linux Technology Center, fubar-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2011-03-01 20:12 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-02-28 13:32 [PATCH] fcoe: correct checking for bonding Jiri Pirko 2011-02-28 17:15 ` Jay Vosburgh 2011-02-28 17:54 ` [Open-FCoE] " Joe Eykholt 2011-03-01 6:37 ` Jiri Pirko 2011-03-01 17:49 ` Joe Eykholt 2011-03-01 18:01 ` Vasu Dev 2011-03-01 18:09 ` Vasu Dev 2011-03-01 18:06 ` Jay Vosburgh 2011-03-01 20:12 ` Jiri Pirko
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).