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. > > > 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 > > -- > > > > Ales Musil > > > > Senior Software Engineer - OVN Core > > > > Red Hat EMEA <https://www.redhat.com> > > > > [email protected] <mailto:[email protected]> > > > > <https://red.ht/sig> > > > > Thanks, Ales -- Ales Musil Senior Software Engineer - OVN Core Red Hat EMEA <https://www.redhat.com> [email protected] <https://red.ht/sig> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
