On Wed, Apr 27, 2022 at 01:36:48PM +0200, Adrian Moreno wrote:
>
>
> On 4/7/22 15:53, lic121 wrote:
> >During the revalidation, mirror could be removed. Instead of crash
> >the process, we can simply skip the deleted mirror.
> >
> >Fixes: ec7ceaed4f3e ("ofproto-dpif: Modularize mirror code.")
> >Signed-off-by: lic121 <[email protected]>
> >---
> > ofproto/ofproto-dpif-xlate.c | 10 +++++++---
> > 1 file changed, 7 insertions(+), 3 deletions(-)
> >
> >diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> >index 5a770f1..33e56c2 100644
> >--- a/ofproto/ofproto-dpif-xlate.c
> >+++ b/ofproto/ofproto-dpif-xlate.c
> >@@ -2142,9 +2142,13 @@ mirror_packet(struct xlate_ctx *ctx, struct xbundle
> >*xbundle,
> > int snaplen;
> > /* Get the details of the mirror represented by the rightmost
> > 1-bit. */
> >- ovs_assert(mirror_get(xbridge->mbridge, raw_ctz(mirrors),
> >- &vlans, &dup_mirrors,
> >- &out, &snaplen, &out_vlan));
> >+ if (!mirror_get(xbridge->mbridge, raw_ctz(mirrors),
> >+ &vlans, &dup_mirrors,
> >+ &out, &snaplen, &out_vlan)) {
> >+ /* Mirror could be deleted during revalidation */
> >+ mirrors = zero_rightmost_1bit(mirrors);
> >+ continue;
> >+ }
> > /* If this mirror selects on the basis of VLAN, and it does not
> > select
>
> I have reproduced the assert and verified this patch fixes it.
>
> The patch looks good to me. My only nit is about the comment, it's a
> bit confusing because this code is executed also in upcall handling,
> not only on revalidation. Something on the lines of the following
> would be a bit more clear IMHO:
>
> /* The mirror got reconfigured before we got to read it's configuration. */
>
> Also, maybe add OVS_UNLIKELY?
Thanks for the review, will address the comments in the next version.
>
> Thanks.
> --
> Adrián Moreno
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
