[ovs-dev] [PATCH] ofproto-dpif: Zero-out subfacet counters when installation fails.

Justin Pettit jpettit at nicira.com
Sat Jul 13 02:04:11 UTC 2013


Okay, I made it more explicit in both and pushed to affected branches.  Thanks for the review and strategy discussion.

--Justin


On Jul 12, 2013, at 5:21 PM, Ben Pfaff <blp at nicira.com> wrote:

> One thing that isn't entirely clear from the commit message and comment is whether we believe this message, when it appears, indicates that there is a bug somewhere. I think the intention is that the message indicates a bug, I'm just saying this to check that I'm right.
> 
> The code change definitely fixes a potential problem and I'm in favor of it.
> 
> Thanks,
> 
> Ben.
> 
> On Jul 12, 2013 5:12 PM, "Justin Pettit" <jpettit at nicira.com> wrote:
> When deleting subfacets, subfacet_uninstall() asserts that the
> subfacet's counters are zero to make sure we don't lose counters.  We
> have seen cases where a subfacet cannot be installed, but the counters
> have values.  This should not happen, since we shouldn't create a
> subfacet if the datapath has a flow.  To prevent asserts, this commit
> zeros out the counters and logs an error, since it's possible for a
> datapath to trigger this.
> 
> Bug #18460.
> 
> Signed-off-by: Justin Pettit <jpettit at nicira.com>
> ---
>  ofproto/ofproto-dpif.c |   10 ++++++++++
>  1 files changed, 10 insertions(+), 0 deletions(-)
> 
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 67e6c7a..8b216e5 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -3760,6 +3760,16 @@ handle_miss_upcalls(struct dpif_backer *backer, struct dpif_upcall *upcalls,
> 
>              COVERAGE_INC(subfacet_install_fail);
> 
> +            /* Zero-out subfacet counters when installation failed, but
> +             * datapath reported hits.  This should not happen, since if
> +             * the datapath flow exists, we should not be attempting to
> +             * create a new subfacet. */
> +            if (subfacet->dp_packet_count || subfacet->dp_byte_count) {
> +                VLOG_ERR_RL(&rl, "failed to install subfacet for which "
> +                            "datapath reported hits");
> +                subfacet->dp_packet_count = subfacet->dp_byte_count = 0;
> +            }
> +
>              subfacet->path = SF_NOT_INSTALLED;
>          }
> 
> --
> 1.7.5.4
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev




More information about the dev mailing list