Thanks Ben. It works. Just a small typo in your patch.

+    port->pp.state = ((port->pp.state & ~OFPUTIL_PS_LINK_DOWN)
+                      | (pp->state & OFPUTIL_PS_LINK_DOWN));

Can you please tell me how is the ofproto_modified() function is
called(what is the event which triggers this).

Thanks,
-Vasu


On Wed, Feb 19, 2014 at 2:45 PM, Ben Pfaff <[email protected]> wrote:

> On Mon, Feb 17, 2014 at 06:58:58PM -0500, Vasu Dasari wrote:
> > On debugging this issue further, I found that, ofproto's STP state bits
> are
> > getting updated correctly because of following call stack:
> >
> > #0  ofproto_port_set_state (port=0x82395c0, state=OFPUTIL_PS_STP_FORWARD)
> >     at ../ofproto/ofproto.c:2267
> > #1  0x0806d659 in update_stp_port_state (ofport=0x82395b8)
> >     at ../ofproto/ofproto-dpif.c:1866
> > #2  0x0806da37 in stp_run (ofproto=0x822c628) at
> > ../ofproto/ofproto-dpif.c:1969
> > #3  0x0806c1ba in run (ofproto_=0x822c630) at
> ../ofproto/ofproto-dpif.c:1286
> > #4  0x0805d51f in ofproto_run (p=0x822c630) at ../ofproto/ofproto.c:1429
> > #5  0x08050ece in bridge_run__ () at ../vswitchd/bridge.c:2275
> > #6  0x08051032 in bridge_run () at ../vswitchd/bridge.c:2326
> > #7  0x08055a99 in main (argc=4, argv=0xbffff224) at
> > ../vswitchd/ovs-vswitchd.c:118
> >
> > but the same STP state is getting overwritten by following call stack,
> at a
> > later point in time:
> >
> > #0  ofport_modified (port=0x822c290, pp=0xbfffefb4) at
> > ../ofproto/ofproto.c:2244
> > #1  0x0805f887 in update_port (ofproto=0x821f090, name=0x8200958
> > "atl0-eth2")
> >     at ../ofproto/ofproto.c:2423
> > #2  0x0805d867 in ofproto_run (p=0x821f090) at ../ofproto/ofproto.c:1495
> > #3  0x08050ece in bridge_run__ () at ../vswitchd/bridge.c:2275
> > #4  0x08051032 in bridge_run () at ../vswitchd/bridge.c:2326
> > #5  0x08055a99 in main (argc=4, argv=0xbffff224) at
> > ../vswitchd/ovs-vswitchd.c:118
> >
> > This is the reason why, I am seeing the "state to be 0".
>
> It looks very much to me like ofport_modified() simply shouldn't be
> messing around with any of the state bits other than
> OFPUTIL_PS_LINK_DOWN.  Can you try this patch?  If it solves the
> problem I'll write it up properly.
>
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index 48e10ca..f544c68 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -2244,7 +2244,8 @@ ofport_modified(struct ofport *port, struct
> ofputil_phy_port *pp)
>      memcpy(port->pp.hw_addr, pp->hw_addr, ETH_ADDR_LEN);
>      port->pp.config = ((port->pp.config & ~OFPUTIL_PC_PORT_DOWN)
>                          | (pp->config & OFPUTIL_PC_PORT_DOWN));
> -    port->pp.state = pp->state;
> +    port->pp.state = ((port->ps.state & ~OFPUTIL_PS_LINK_DOWN)
> +                      | (pp->state & OFPUTIL_PS_LINK_DOWN));
>      port->pp.curr = pp->curr;
>      port->pp.advertised = pp->advertised;
>      port->pp.supported = pp->supported;
>
_______________________________________________
discuss mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/discuss

Reply via email to