[ovs-dev] [PATCH 3/3] bond: Fix comment on bond_entry::tag

Zoltan Kiss zoltan.kiss at citrix.com
Mon Jan 7 22:39:56 UTC 2013


On 07/01/13 19:33, Ben Pfaff wrote:
> On Sat, Jan 05, 2013 at 09:42:16PM +0000, Zoltan Kiss wrote:
>> The hash entry tag connects to facet(s), not slaves.
>>
>> Signed-off-by: Zoltan Kiss <zoltan.kiss at citrix.com>
>> ---
>>   lib/bond.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/lib/bond.c b/lib/bond.c
>> index a95c9b0..2446d7e 100644
>> --- a/lib/bond.c
>> +++ b/lib/bond.c
>> @@ -50,7 +50,7 @@ VLOG_DEFINE_THIS_MODULE(bond);
>>   struct bond_entry {
>>       struct bond_slave *slave;   /* Assigned slave, NULL if unassigned. */
>>       uint64_t tx_bytes;          /* Count of bytes recently transmitted. */
>> -    tag_type tag;               /* Tag for entry<->slave association. */
>> +    tag_type tag;               /* Tag for entry<->facet association. */
>>       struct list list_node;      /* In bond_slave's 'entries' list. */
>>   };
>
> I think that the comment is actually correct.  The tag changes
> whenever a bond_entry moves from one bond_slave to another.  It goes
> without saying that this is also tied to a facet, since that's only
> actual use for tags.
>
> Some other comments in bond.c are clearly wrong.  I'll send out a fix.
>
My understanding is that 3 kind of objects have tags: facets, slaves and 
hash entries. Slave and hash tags linking them to facet(s) tags, but 
although hash tags changing when rebalance happens and new slave chosen, 
they have nothing to do about the slave's tag. My original assumption 
was that the tag link the hash to the slave, and the first fix I've sent 
reflected this.
But your commit message here:

http://openvswitch.org/cgi-bin/gitweb.cgi?p=openvswitch;a=commitdiff;h=865f22b3b3cb953c48ed30dd21f16ea3dd53f04c

"According to the hash value for a flow, to make it easy to invalidate 
all of the flows that hash into the same bucket."

... suggest me that although the hash tag changes at the moment only if 
rebalance happens, that might be not true in the future. Or do I 
misunderstand something in the concept?

Zoli



More information about the dev mailing list