Ben Pfaff <[email protected]> writes: > On Fri, Jan 31, 2020 at 03:34:02PM -0500, Aaron Conole wrote: >> When commit a0baa7dfa4fe ("connmgr: Make treatment of active and passive >> connections more uniform") was applied, it didn't take into account >> that a reconfiguration of the allowed_versions setting needs to >> propagate to the rconn and pvconn (and lower connection) elements. >> >> This change inelegantly deletes and recreates these pvconn and rconn >> elements. There's probably better ways of doing this (like maybe >> adding the knob to rconn/vconn and kicking off a resync?) but I don't >> know the openflow spec well enough to know whether it's possible or >> supported. >> >> A new test is added to ensure we don't break the behavior again. >> >> Fixes: a0baa7dfa4fe ("connmgr: Make treatment of active and passive >> connections more uniform") >> >> Signed-off-by: Aaron Conole <[email protected]> > > I spent some time working to make it possible to change this kind of > thing midstream without deleting and recreating everything. It > unfortunately gets bogged down in nasty corner cases, so I don't want to > pursue it unless it turns out to be important. > > I think this patch does introduce a problem, though. The ofservice code > assumes in a few places that an ofservice has a pvconn or an rconn. If > the pvconn goes away and there's no rconn, then we'll get crashes I > think. See ofservice_run(), for example. The code needs to avoid that.
Okay, I'll take a closer look. One method I can thing to use for this would be to rcu-ify the pvconn and rconn pointers in the ofservice struct. Then swap in a new pointer, and defer release to the next sync period. This means that at all times there should be a valid rconn and pvconn pointer. BUT I don't know if this will work for the pvconn (because I don't know if it will run into some kind of resource conflict with the listening socket), but maybe it will need to have SO_REUSEPORT added to the list of socket options in inet_open_passive (for the pvconn). I guess for rconn I would expect that the remote endpoint should have some listen backlog, so I don't think anything should be needed in that case. WDYT? _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
