| Message ID | 20170110235403.15331-2-joe@ovn.org |
|---|---|
| State | Accepted |
| Headers | show |
In the worst case this patch will introduce rate-limited logging for other states than UKEY_OPERATIONAL that may want to transition onto themselves, so: Acked-by: Jarno Rajahalme <jarno@ovn.org> > On Jan 10, 2017, at 3:54 PM, Joe Stringer <joe@ovn.org> wrote: > > For most ukey transition states, only one thread should be responsible > for transitioning the ukey into the new state. If another thread > attempts to transition the ukey into the same state (for instance, > evicting the datapath flow or deleting the ukey), then it is likely > performing additional work which should only happen once. Log all cases > of ukey transition into the current state, except for UKEY_OPERATIONAL > -> UKEY_OPERATIONAL which regularly occurs when revalidating ukeys. > > Signed-off-by: Joe Stringer <joe@ovn.org> > --- > ofproto/ofproto-dpif-upcall.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c > index 1ffeaabf7d8e..660383faee1c 100644 > --- a/ofproto/ofproto-dpif-upcall.c > +++ b/ofproto/ofproto-dpif-upcall.c > @@ -1672,7 +1672,7 @@ transition_ukey(struct udpif_key *ukey, enum ukey_state dst) > OVS_REQUIRES(ukey->mutex) > { > ovs_assert(dst >= ukey->state); > - if (ukey->state == dst) { > + if (ukey->state == dst && dst == UKEY_OPERATIONAL) { > return; > } > > -- > 2.10.2 >
On 10 January 2017 at 16:02, Jarno Rajahalme <jarno@ovn.org> wrote: > In the worst case this patch will introduce rate-limited logging for other states than UKEY_OPERATIONAL that may want to transition onto themselves, so: > > Acked-by: Jarno Rajahalme <jarno@ovn.org> Thanks. I have reasonable confidence this shouldn't happen, but I agree that worst case we'll just seeing some spurious, rate-limited logging. If this happens, we can revise the condition here to include those other expected conditions. Applied to master.
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c index 1ffeaabf7d8e..660383faee1c 100644 --- a/ofproto/ofproto-dpif-upcall.c +++ b/ofproto/ofproto-dpif-upcall.c @@ -1672,7 +1672,7 @@ transition_ukey(struct udpif_key *ukey, enum ukey_state dst) OVS_REQUIRES(ukey->mutex) { ovs_assert(dst >= ukey->state); - if (ukey->state == dst) { + if (ukey->state == dst && dst == UKEY_OPERATIONAL) { return; }
For most ukey transition states, only one thread should be responsible for transitioning the ukey into the new state. If another thread attempts to transition the ukey into the same state (for instance, evicting the datapath flow or deleting the ukey), then it is likely performing additional work which should only happen once. Log all cases of ukey transition into the current state, except for UKEY_OPERATIONAL -> UKEY_OPERATIONAL which regularly occurs when revalidating ukeys. Signed-off-by: Joe Stringer <joe@ovn.org> --- ofproto/ofproto-dpif-upcall.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)