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

[Ben Pfaff]

On Mon, Jan 07, 2013 at 10:39:56PM +0000, Zoltan Kiss wrote:
> 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?

Somehow, I'm not sure how, I'm just not following the question here.
Maybe you can rephrase it, or step back somehow.