On 4/2/24 16:32, Ales Musil wrote: > On Tue, Apr 2, 2024 at 4:25 PM Ilya Maximets <[email protected]> wrote: > >> On 4/2/24 16:17, Ales Musil wrote: >>> >>> >>> On Tue, Apr 2, 2024 at 4:03 PM Ilya Maximets <[email protected] >> <mailto:[email protected]>> wrote: >>> >>> On 4/2/24 15:36, Ales Musil wrote: >>> > >>> > >>> > On Tue, Apr 2, 2024 at 3:02 PM Ilya Maximets <[email protected] >> <mailto:[email protected]> <mailto:[email protected] <mailto: >> [email protected]>>> wrote: >>> > >>> > On 3/28/24 09:53, Ales Musil wrote: >>> > > The br-int connection is hardcoded to use unix socket, which >> requires >>> > > for the socket to be visible for ovn-controller. This is >> achievable in >>> > > container by mounting the socket, but in turn the container >> requires >>> > > additional privileges. >>> > > >>> > > Add option to ovn-controller that allows to specify remote >> target for >>> > > br-int. This gives the user possibility to connect to br-int >> in different >>> > > manner than unix socket, defaulting to the unix socket when >> not specified. >>> > > In addition, there is an option to specify inactivity probe >> for this >>> > > connection, disabled by default. >>> > > >>> > > Reported-at: https://issues.redhat.com/browse/FDP-243 < >> https://issues.redhat.com/browse/FDP-243> < >> https://issues.redhat.com/browse/FDP-243 < >> https://issues.redhat.com/browse/FDP-243>> >>> > > Signed-off-by: Ales Musil <[email protected] <mailto: >> [email protected]> <mailto:[email protected] <mailto:[email protected]>>> >>> > > --- >>> > > NEWS | 6 +++ >>> > > controller/ofctrl.c | 10 +---- >>> > > controller/ofctrl.h | 5 ++- >>> > > controller/ovn-controller.8.xml | 12 ++++++ >>> > > controller/ovn-controller.c | 68 >> ++++++++++++++++++++++++++------- >>> > > controller/pinctrl.c | 56 >> ++++++--------------------- >>> > > controller/pinctrl.h | 6 ++- >>> > > controller/statctrl.c | 66 >> ++++++-------------------------- >>> > > controller/statctrl.h | 3 +- >>> > > include/ovn/features.h | 2 +- >>> > > lib/features.c | 35 +++++------------ >>> > > lib/ovn-util.c | 26 +++++++++++++ >>> > > lib/ovn-util.h | 4 ++ >>> > > lib/test-ovn-features.c | 6 +-- >>> > > tests/ovn-controller.at <http://ovn-controller.at> < >> http://ovn-controller.at <http://ovn-controller.at>> | 31 >> +++++++++++++++ >>> > > utilities/ovn-ctl | 10 +++++ >>> > > 16 files changed, 192 insertions(+), 154 deletions(-) >>> > > >>> > > diff --git a/NEWS b/NEWS >>> > > index 4d6ebea89..4979bb806 100644 >>> > > --- a/NEWS >>> > > +++ b/NEWS >>> > > @@ -12,6 +12,12 @@ Post v24.03.0 >>> > > flow table id. >>> > > "lflow-stage-to-oftable STAGE_NAME" that converts stage >> name into OpenFlow >>> > > table id. >>> > > + - Add option to ovn-controller called >> "--br-int-remote=REMOTE" that allows >>> > > + to specify connection method to integration bridge for >> ovn-controller, >>> > > + defaulting to the unix socket. >>> > > + - Add option to ovn-controller called >> "--br-int-probe-interval=INTERVAL" >>> > > + that sets probe interval for integration bridge >> connection, >>> > > + disabled by default. >>> > >>> > I didn't review the code, but I don't think the names should >> be changed. >>> > There is nothing user-facing called "br-int" and any bridge >> name can be >>> > chosen for a deployment. It should be called >> "ovn-bridge-mgmt-remote" or >>> > something like that. >>> > >>> > >>> > I'm fine either way, I just went with br-int because the code >> around that is actually >>> > called br-int even if we have different management bridge name. >>> >>> The point is - users do not know how the code looks like >>> and they may have nothing called 'br-int' in their setup. >>> >>> >>> Sure that is fine I can change the user facing part. >>> >>> >>> >>> Also, maybe these should be a database configuration instead >>> of command line arguments? Alongside the 'ovn-bridge'. >>> >>> >>> So having in DB means we have to react to any change, the question is, >> do we expect >>> uset to change this configuration often? >> >> Do we react to 'ovn-bridge' changes? I guess, ovn-controller can react to >> connection changes the same way. This configuration should not change >> frequently, IMO. >> > > I'll let others chime in, again I don't have a strong preference for this > one, it can be either of those. >
If I may, I vote for storing the OF connection method as external ID in the OVS database. I agree with Ilya, the "br-int" bridge name (ovn-bridge) is already there, it makes the most sense to keep everything together in one place. > >> >>> Also "ovs-database" is also just an argument to the ovn-controller. >> >> Database connection and OF connection are different things, and we have >> to know how to connect to the database. The model until now was to have >> a single command line argument - database connection method - and store >> all the other configuration in the database. >> >>> >>> >>> >>> Best regards, Ilya Maximets. >>> >>> >>> Thanks, >>> Ales >>> -- Regards, Dumitru _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
