[ovs-dev] [PATCH] ofproto-dpif-xlate: Support BFD, CFM, carrier, and LACP for port liveness.

[Niels van Adrichem]

Sorry for my late reply, I am on a different timezone (Amsterdam time,
GMT+2).

It is good to bring back the amounts of operations in
odp_port_is_alive() as it is checked at every packet, hence a single
variable containing a summary of all combinations is the right way.

To answer your question, the patch works well. I toyed around with
min_rx and min_tx a bit and it also appears to change output ports fast
at first sight.

Best regards,
Niels


On do, 2014-10-16 at 15:29 -0700, Ben Pfaff wrote:
> On Thu, Oct 16, 2014 at 03:19:03PM -0700, Alex Wang wrote:
> > On Thu, Oct 16, 2014 at 3:00 PM, Ben Pfaff <blp at nicira.com> wrote:
> > 
> > > This is simpler and shorter than handling each of these by itself.
> > >
> > > CC: Niels van Adrichem <N.L.M.vanAdrichem at tudelft.nl>
> > > Suggested-by: Alex Wang <alexw at nicira.com>
> > > Signed-off-by: Ben Pfaff <blp at nicira.com>
> > > ---
> > >  ofproto/ofproto-dpif-xlate.c |   17 ++---------------
> > >  1 file changed, 2 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> > > index 48576ad..d3ad62a 100644
> > > --- a/ofproto/ofproto-dpif-xlate.c
> > > +++ b/ofproto/ofproto-dpif-xlate.c
> > > @@ -1246,21 +1246,8 @@ ofp_port_to_odp_port(const struct xbridge *xbridge,
> > > ofp_port_t ofp_port)
> > >  static bool
> > >  odp_port_is_alive(const struct xlate_ctx *ctx, ofp_port_t ofp_port)
> > >  {
> > > -    struct xport *xport;
> > > -    struct bfd *bfd;
> > > -
> > > -    xport = get_ofp_port(ctx->xbridge, ofp_port);
> > > -    if (!xport || xport->config & OFPUTIL_PC_PORT_DOWN ||
> > > -        xport->state & OFPUTIL_PS_LINK_DOWN) {
> > > -        return false;
> > > -    }
> > 
> > 
> > 
> > > -    bfd = xport->bfd;
> > > -    if (bfd && !bfd_forwarding(bfd)) {
> > > -        return false;
> > > -    }
> > > -
> > > -    return true;
> > > +    struct xport *xport = get_ofp_port(ctx->xbridge, ofp_port);
> > > +    return xport && xport->may_enable;
> > >  }
> > >
> > >  static struct ofputil_bucket *
> > > --
> > > 1.7.10.4
> > >
> > >
> > This looks very clean, port_run() does everything! including the
> > netdev_get_carrier() check~
> > 
> > 
> > Acked-by: Alex Wang <alexw at nicira.com>
> 
> Thanks.  I decided to apply it right away because this looks like the
> right approach to me too.
> 
> Niels, please do test it and let us know if it does not work for you.
> 
> Thanks,
> 
> Ben.