| Message ID | 20180427170030.5180-1-kumaranand@vmware.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,v2] datapath-windows: Prevent ct-counters from getting redundantly incremented | expand |
> On 27 Apr 2018, at 20:00, Anand Kumar <kumaranand@vmware.com> wrote: > > The conntrack-counters ought to be incremented only if it's a new lookup > or if it's recirculated through a different zone for the first time. > > Signed-off-by: Anand Kumar <kumaranand@vmware.com> > --- > datapath-windows/ovsext/Conntrack.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/datapath-windows/ovsext/Conntrack.c b/datapath-windows/ovsext/Conntrack.c > index 678bedb..add1491 100644 > --- a/datapath-windows/ovsext/Conntrack.c > +++ b/datapath-windows/ovsext/Conntrack.c > @@ -886,10 +886,11 @@ OvsCtExecute_(OvsForwardingContext *fwdCtx, > return NDIS_STATUS_RESOURCES; > } > > - /* Increment the counters soon after the lookup, since we set ct.state > - * to OVS_CS_F_TRACKED after processing the ct entry. > + /* Increment stats for the entry if it wasn't tracked previously or > + * if they are on different zones > */ > - if (entry && (!(key->ct.state & OVS_CS_F_TRACKED))) { > + if (entry && (entry->key.zone != key->ct.zone || > + (!(key->ct.state & OVS_CS_F_TRACKED)))) { > OvsCtIncrementCounters(entry, ctx.reply, curNbl); > } > > -- > 2.9.3.windows.1 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev <https://mail.openvswitch.org/mailman/listinfo/ovs-dev> Acked-by: Alin Gabriel Serdean <aserdean@ovn.org <mailto:aserdean@ovn.org>>
On Fri, Apr 27, 2018 at 08:03:43PM +0300, Alin Gabriel Serdean wrote: > > > On 27 Apr 2018, at 20:00, Anand Kumar <kumaranand@vmware.com> wrote: > > > > The conntrack-counters ought to be incremented only if it's a new lookup > > or if it's recirculated through a different zone for the first time. > > > > Signed-off-by: Anand Kumar <kumaranand@vmware.com> > > --- > > datapath-windows/ovsext/Conntrack.c | 7 ++++--- > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > diff --git a/datapath-windows/ovsext/Conntrack.c b/datapath-windows/ovsext/Conntrack.c > > index 678bedb..add1491 100644 > > --- a/datapath-windows/ovsext/Conntrack.c > > +++ b/datapath-windows/ovsext/Conntrack.c > > @@ -886,10 +886,11 @@ OvsCtExecute_(OvsForwardingContext *fwdCtx, > > return NDIS_STATUS_RESOURCES; > > } > > > > - /* Increment the counters soon after the lookup, since we set ct.state > > - * to OVS_CS_F_TRACKED after processing the ct entry. > > + /* Increment stats for the entry if it wasn't tracked previously or > > + * if they are on different zones > > */ > > - if (entry && (!(key->ct.state & OVS_CS_F_TRACKED))) { > > + if (entry && (entry->key.zone != key->ct.zone || > > + (!(key->ct.state & OVS_CS_F_TRACKED)))) { > > OvsCtIncrementCounters(entry, ctx.reply, curNbl); > > } > > > > -- > > 2.9.3.windows.1 > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev <https://mail.openvswitch.org/mailman/listinfo/ovs-dev> > > Acked-by: Alin Gabriel Serdean <aserdean@ovn.org <mailto:aserdean@ovn.org>> Do you want to apply this to the tree?
> On 28 Apr 2018, at 21:26, Ben Pfaff <blp@ovn.org> wrote: > > On Fri, Apr 27, 2018 at 08:03:43PM +0300, Alin Gabriel Serdean wrote: >> >>> On 27 Apr 2018, at 20:00, Anand Kumar <kumaranand@vmware.com> wrote: >>> >>> The conntrack-counters ought to be incremented only if it's a new lookup >>> or if it's recirculated through a different zone for the first time. >>> >>> Signed-off-by: Anand Kumar <kumaranand@vmware.com> >>> --- >>> datapath-windows/ovsext/Conntrack.c | 7 ++++--- >>> 1 file changed, 4 insertions(+), 3 deletions(-) >>> >>> diff --git a/datapath-windows/ovsext/Conntrack.c b/datapath-windows/ovsext/Conntrack.c >>> index 678bedb..add1491 100644 >>> --- a/datapath-windows/ovsext/Conntrack.c >>> +++ b/datapath-windows/ovsext/Conntrack.c >>> @@ -886,10 +886,11 @@ OvsCtExecute_(OvsForwardingContext *fwdCtx, >>> return NDIS_STATUS_RESOURCES; >>> } >>> >>> - /* Increment the counters soon after the lookup, since we set ct.state >>> - * to OVS_CS_F_TRACKED after processing the ct entry. >>> + /* Increment stats for the entry if it wasn't tracked previously or >>> + * if they are on different zones >>> */ >>> - if (entry && (!(key->ct.state & OVS_CS_F_TRACKED))) { >>> + if (entry && (entry->key.zone != key->ct.zone || >>> + (!(key->ct.state & OVS_CS_F_TRACKED)))) { >>> OvsCtIncrementCounters(entry, ctx.reply, curNbl); >>> } >>> >>> -- >>> 2.9.3.windows.1 >>> >>> _______________________________________________ >>> dev mailing list >>> dev@openvswitch.org >>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev <https://mail.openvswitch.org/mailman/listinfo/ovs-dev> >> >> Acked-by: Alin Gabriel Serdean <aserdean@ovn.org <mailto:aserdean@ovn.org>> > > Do you want to apply this to the tree? I will apply them thanks for asking Ben. I am on vacation this week so response will be increased :). > _______________________________________________ > dev mailing list > dev@openvswitch.org <mailto:dev@openvswitch.org> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev <https://mail.openvswitch.org/mailman/listinfo/ovs-dev>
> On 27 Apr 2018, at 20:00, Anand Kumar <kumaranand@vmware.com> wrote: > > The conntrack-counters ought to be incremented only if it's a new lookup > or if it's recirculated through a different zone for the first time. > > Signed-off-by: Anand Kumar <kumaranand@vmware.com> Applied on master. Thanks Anand!
On Mon, Apr 30, 2018 at 11:26:52PM +0300, Alin Gabriel Serdean wrote: > > > On 28 Apr 2018, at 21:26, Ben Pfaff <blp@ovn.org> wrote: > > > > On Fri, Apr 27, 2018 at 08:03:43PM +0300, Alin Gabriel Serdean wrote: > >> > >>> On 27 Apr 2018, at 20:00, Anand Kumar <kumaranand@vmware.com> wrote: > >>> > >>> The conntrack-counters ought to be incremented only if it's a new lookup > >>> or if it's recirculated through a different zone for the first time. > >>> > >>> Signed-off-by: Anand Kumar <kumaranand@vmware.com> > >>> --- > >>> datapath-windows/ovsext/Conntrack.c | 7 ++++--- > >>> 1 file changed, 4 insertions(+), 3 deletions(-) > >>> > >>> diff --git a/datapath-windows/ovsext/Conntrack.c b/datapath-windows/ovsext/Conntrack.c > >>> index 678bedb..add1491 100644 > >>> --- a/datapath-windows/ovsext/Conntrack.c > >>> +++ b/datapath-windows/ovsext/Conntrack.c > >>> @@ -886,10 +886,11 @@ OvsCtExecute_(OvsForwardingContext *fwdCtx, > >>> return NDIS_STATUS_RESOURCES; > >>> } > >>> > >>> - /* Increment the counters soon after the lookup, since we set ct.state > >>> - * to OVS_CS_F_TRACKED after processing the ct entry. > >>> + /* Increment stats for the entry if it wasn't tracked previously or > >>> + * if they are on different zones > >>> */ > >>> - if (entry && (!(key->ct.state & OVS_CS_F_TRACKED))) { > >>> + if (entry && (entry->key.zone != key->ct.zone || > >>> + (!(key->ct.state & OVS_CS_F_TRACKED)))) { > >>> OvsCtIncrementCounters(entry, ctx.reply, curNbl); > >>> } > >>> > >>> -- > >>> 2.9.3.windows.1 > >>> > >>> _______________________________________________ > >>> dev mailing list > >>> dev@openvswitch.org > >>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev <https://mail.openvswitch.org/mailman/listinfo/ovs-dev> > >> > >> Acked-by: Alin Gabriel Serdean <aserdean@ovn.org <mailto:aserdean@ovn.org>> > > > > Do you want to apply this to the tree? > > I will apply them thanks for asking Ben. I am on vacation this week so response will be increased :). I'll be in Copenhagen this week (leaving for the airport in about 90 minutes actually), so me too.
> On 30 Apr 2018, at 23:36, Ben Pfaff <blp@ovn.org> wrote: > > On Mon, Apr 30, 2018 at 11:26:52PM +0300, Alin Gabriel Serdean wrote: >> >>> On 28 Apr 2018, at 21:26, Ben Pfaff <blp@ovn.org> wrote: >>> >>> On Fri, Apr 27, 2018 at 08:03:43PM +0300, Alin Gabriel Serdean wrote: >>>> >>>>> On 27 Apr 2018, at 20:00, Anand Kumar <kumaranand@vmware.com> wrote: >>>>> >>>>> The conntrack-counters ought to be incremented only if it's a new lookup >>>>> or if it's recirculated through a different zone for the first time. >>>>> >>>>> Signed-off-by: Anand Kumar <kumaranand@vmware.com> >>>>> --- >>>>> datapath-windows/ovsext/Conntrack.c | 7 ++++--- >>>>> 1 file changed, 4 insertions(+), 3 deletions(-) >>>>> >>>>> diff --git a/datapath-windows/ovsext/Conntrack.c b/datapath-windows/ovsext/Conntrack.c >>>>> index 678bedb..add1491 100644 >>>>> --- a/datapath-windows/ovsext/Conntrack.c >>>>> +++ b/datapath-windows/ovsext/Conntrack.c >>>>> @@ -886,10 +886,11 @@ OvsCtExecute_(OvsForwardingContext *fwdCtx, >>>>> return NDIS_STATUS_RESOURCES; >>>>> } >>>>> >>>>> - /* Increment the counters soon after the lookup, since we set ct.state >>>>> - * to OVS_CS_F_TRACKED after processing the ct entry. >>>>> + /* Increment stats for the entry if it wasn't tracked previously or >>>>> + * if they are on different zones >>>>> */ >>>>> - if (entry && (!(key->ct.state & OVS_CS_F_TRACKED))) { >>>>> + if (entry && (entry->key.zone != key->ct.zone || >>>>> + (!(key->ct.state & OVS_CS_F_TRACKED)))) { >>>>> OvsCtIncrementCounters(entry, ctx.reply, curNbl); >>>>> } >>>>> >>>>> -- >>>>> 2.9.3.windows.1 >>>>> >>>>> _______________________________________________ >>>>> dev mailing list >>>>> dev@openvswitch.org >>>>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev <https://mail.openvswitch.org/mailman/listinfo/ovs-dev> >>>> >>>> Acked-by: Alin Gabriel Serdean <aserdean@ovn.org <mailto:aserdean@ovn.org>> >>> >>> Do you want to apply this to the tree? >> >> I will apply them thanks for asking Ben. I am on vacation this week so response will be increased :). > > I'll be in Copenhagen this week (leaving for the airport in about 90 > minutes actually), so me too. I saw that you are going to kubecon! I hope you enjoy your time over there :). I hope you and Alin Balutoiu can come and say hi! - Alin.
On Mon, Apr 30, 2018 at 11:47:43PM +0300, Alin Gabriel Serdean wrote: > > > > On 30 Apr 2018, at 23:36, Ben Pfaff <blp@ovn.org> wrote: > > > > On Mon, Apr 30, 2018 at 11:26:52PM +0300, Alin Gabriel Serdean wrote: > >> > >>> On 28 Apr 2018, at 21:26, Ben Pfaff <blp@ovn.org> wrote: > >>> > >>> On Fri, Apr 27, 2018 at 08:03:43PM +0300, Alin Gabriel Serdean wrote: > >>>> > >>>>> On 27 Apr 2018, at 20:00, Anand Kumar <kumaranand@vmware.com> wrote: > >>>>> > >>>>> The conntrack-counters ought to be incremented only if it's a new lookup > >>>>> or if it's recirculated through a different zone for the first time. > >>>>> > >>>>> Signed-off-by: Anand Kumar <kumaranand@vmware.com> > >>>>> --- > >>>>> datapath-windows/ovsext/Conntrack.c | 7 ++++--- > >>>>> 1 file changed, 4 insertions(+), 3 deletions(-) > >>>>> > >>>>> diff --git a/datapath-windows/ovsext/Conntrack.c b/datapath-windows/ovsext/Conntrack.c > >>>>> index 678bedb..add1491 100644 > >>>>> --- a/datapath-windows/ovsext/Conntrack.c > >>>>> +++ b/datapath-windows/ovsext/Conntrack.c > >>>>> @@ -886,10 +886,11 @@ OvsCtExecute_(OvsForwardingContext *fwdCtx, > >>>>> return NDIS_STATUS_RESOURCES; > >>>>> } > >>>>> > >>>>> - /* Increment the counters soon after the lookup, since we set ct.state > >>>>> - * to OVS_CS_F_TRACKED after processing the ct entry. > >>>>> + /* Increment stats for the entry if it wasn't tracked previously or > >>>>> + * if they are on different zones > >>>>> */ > >>>>> - if (entry && (!(key->ct.state & OVS_CS_F_TRACKED))) { > >>>>> + if (entry && (entry->key.zone != key->ct.zone || > >>>>> + (!(key->ct.state & OVS_CS_F_TRACKED)))) { > >>>>> OvsCtIncrementCounters(entry, ctx.reply, curNbl); > >>>>> } > >>>>> > >>>>> -- > >>>>> 2.9.3.windows.1 > >>>>> > >>>>> _______________________________________________ > >>>>> dev mailing list > >>>>> dev@openvswitch.org > >>>>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev <https://mail.openvswitch.org/mailman/listinfo/ovs-dev> > >>>> > >>>> Acked-by: Alin Gabriel Serdean <aserdean@ovn.org <mailto:aserdean@ovn.org>> > >>> > >>> Do you want to apply this to the tree? > >> > >> I will apply them thanks for asking Ben. I am on vacation this week so response will be increased :). > > > > I'll be in Copenhagen this week (leaving for the airport in about 90 > > minutes actually), so me too. > > I saw that you are going to kubecon! I hope you enjoy your time over there :). > > I hope you and Alin Balutoiu can come and say hi! That's a good idea. Alin B., will you be at the Cloudbase booth? I'm going to be at the VMware booth myself Wednesday 3:30-4:30 and Thursday 4:30-5:30 for "open source office hours".
diff --git a/datapath-windows/ovsext/Conntrack.c b/datapath-windows/ovsext/Conntrack.c index 678bedb..add1491 100644 --- a/datapath-windows/ovsext/Conntrack.c +++ b/datapath-windows/ovsext/Conntrack.c @@ -886,10 +886,11 @@ OvsCtExecute_(OvsForwardingContext *fwdCtx, return NDIS_STATUS_RESOURCES; } - /* Increment the counters soon after the lookup, since we set ct.state - * to OVS_CS_F_TRACKED after processing the ct entry. + /* Increment stats for the entry if it wasn't tracked previously or + * if they are on different zones */ - if (entry && (!(key->ct.state & OVS_CS_F_TRACKED))) { + if (entry && (entry->key.zone != key->ct.zone || + (!(key->ct.state & OVS_CS_F_TRACKED)))) { OvsCtIncrementCounters(entry, ctx.reply, curNbl); }
The conntrack-counters ought to be incremented only if it's a new lookup or if it's recirculated through a different zone for the first time. Signed-off-by: Anand Kumar <kumaranand@vmware.com> --- datapath-windows/ovsext/Conntrack.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)