[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