| Message ID | 20160701093126.10453-3-mchandras@suse.de |
|---|---|
| State | Changes Requested |
| Headers | show |
Markos Chandras <mchandras@suse.de> writes: > The ovs-ctl script is used to launch ovs-vswitchd among other things. > However it does not make it possible to pass DPDK options to the > daemon. We fix this by explicitly looking and extracting the DPDK > options from the command line which is then reconstructed so it can be > parsed by ovs-ctl as usual. The DPDK options are later passed to > ovs-vswitchd. > > Signed-off-by: Markos Chandras <mchandras@suse.de> > --- > utilities/ovs-ctl.in | 34 ++++++++++++++++++++++++++++++++-- > 1 file changed, 32 insertions(+), 2 deletions(-) > > diff --git a/utilities/ovs-ctl.in b/utilities/ovs-ctl.in > index 8ec825b..b4e7bf1 100755 > --- a/utilities/ovs-ctl.in > +++ b/utilities/ovs-ctl.in > @@ -230,7 +230,12 @@ do_start_forwarding () { > fi > > # Start ovs-vswitchd. > - set ovs-vswitchd unix:"$DB_SOCK" > + set ovs-vswitchd > + # DPDK options are expected to be at the beginning of > + # the command line arguments. Add '--' to mark the end > + # of the DPDK options. > + [ -n "$DPDK_OPTS" ] && set -- "$@" $DPDK_OPTS "--" > + set "$@" unix:"$DB_SOCK" NAK - this doesn't work on the latest version of Open vSwitch.
On 07/02/2016 03:22 AM, Aaron Conole wrote: > Markos Chandras <mchandras@suse.de> writes: > >> The ovs-ctl script is used to launch ovs-vswitchd among other things. >> However it does not make it possible to pass DPDK options to the >> daemon. We fix this by explicitly looking and extracting the DPDK >> options from the command line which is then reconstructed so it can be >> parsed by ovs-ctl as usual. The DPDK options are later passed to >> ovs-vswitchd. >> >> Signed-off-by: Markos Chandras <mchandras@suse.de> >> --- >> utilities/ovs-ctl.in | 34 ++++++++++++++++++++++++++++++++-- >> 1 file changed, 32 insertions(+), 2 deletions(-) >> >> diff --git a/utilities/ovs-ctl.in b/utilities/ovs-ctl.in >> index 8ec825b..b4e7bf1 100755 >> --- a/utilities/ovs-ctl.in >> +++ b/utilities/ovs-ctl.in >> @@ -230,7 +230,12 @@ do_start_forwarding () { >> fi >> >> # Start ovs-vswitchd. >> - set ovs-vswitchd unix:"$DB_SOCK" >> + set ovs-vswitchd >> + # DPDK options are expected to be at the beginning of >> + # the command line arguments. Add '--' to mark the end >> + # of the DPDK options. >> + [ -n "$DPDK_OPTS" ] && set -- "$@" $DPDK_OPTS "--" >> + set "$@" unix:"$DB_SOCK" > > NAK - this doesn't work on the latest version of Open vSwitch. > Oh yes right. I only tested that with 2.5.0 but now I see that even the 2.5 branch has changed in that regard.
Markos Chandras <mchandras@suse.de> writes: > On 07/02/2016 03:22 AM, Aaron Conole wrote: >> Markos Chandras <mchandras@suse.de> writes: >> >>> The ovs-ctl script is used to launch ovs-vswitchd among other things. >>> However it does not make it possible to pass DPDK options to the >>> daemon. We fix this by explicitly looking and extracting the DPDK >>> options from the command line which is then reconstructed so it can be >>> parsed by ovs-ctl as usual. The DPDK options are later passed to >>> ovs-vswitchd. >>> >>> Signed-off-by: Markos Chandras <mchandras@suse.de> >>> --- >>> utilities/ovs-ctl.in | 34 ++++++++++++++++++++++++++++++++-- >>> 1 file changed, 32 insertions(+), 2 deletions(-) >>> >>> diff --git a/utilities/ovs-ctl.in b/utilities/ovs-ctl.in >>> index 8ec825b..b4e7bf1 100755 >>> --- a/utilities/ovs-ctl.in >>> +++ b/utilities/ovs-ctl.in >>> @@ -230,7 +230,12 @@ do_start_forwarding () { >>> fi >>> >>> # Start ovs-vswitchd. >>> - set ovs-vswitchd unix:"$DB_SOCK" >>> + set ovs-vswitchd >>> + # DPDK options are expected to be at the beginning of >>> + # the command line arguments. Add '--' to mark the end >>> + # of the DPDK options. >>> + [ -n "$DPDK_OPTS" ] && set -- "$@" $DPDK_OPTS "--" >>> + set "$@" unix:"$DB_SOCK" >> >> NAK - this doesn't work on the latest version of Open vSwitch. >> > > Oh yes right. I only tested that with 2.5.0 but now I see that even the > 2.5 branch has changed in that regard. Oh, this is for 2.5; I'm not sure about it then. I don't know if there is such a process to write commits for specific releases, but since 2.5 is an LTS release it might make sense. As far as I understand, 2.5 will not backport the dpdk database changes. It may acceptable to add something a bit more generic though. I'm thinking there could be some variables as: OVSDB_SERVER_EXTRA OVS_VSWITCHD_EXTRA and those could be passed to the daemons. Such a commit might be backportable(?), and would be easy to use more generically. IE: I could use it to pre-set log levels, or pass 'special' options to the daemon (and the dpdk options could be passed that way, as well). Could that be an acceptable alternative?
On 07/05/2016 03:08 PM, Aaron Conole wrote: > Markos Chandras <mchandras@suse.de> writes: > >> On 07/02/2016 03:22 AM, Aaron Conole wrote: >>> Markos Chandras <mchandras@suse.de> writes: >>> >>>> The ovs-ctl script is used to launch ovs-vswitchd among other things. >>>> However it does not make it possible to pass DPDK options to the >>>> daemon. We fix this by explicitly looking and extracting the DPDK >>>> options from the command line which is then reconstructed so it can be >>>> parsed by ovs-ctl as usual. The DPDK options are later passed to >>>> ovs-vswitchd. >>>> >>>> Signed-off-by: Markos Chandras <mchandras@suse.de> >>>> --- >>>> utilities/ovs-ctl.in | 34 ++++++++++++++++++++++++++++++++-- >>>> 1 file changed, 32 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/utilities/ovs-ctl.in b/utilities/ovs-ctl.in >>>> index 8ec825b..b4e7bf1 100755 >>>> --- a/utilities/ovs-ctl.in >>>> +++ b/utilities/ovs-ctl.in >>>> @@ -230,7 +230,12 @@ do_start_forwarding () { >>>> fi >>>> >>>> # Start ovs-vswitchd. >>>> - set ovs-vswitchd unix:"$DB_SOCK" >>>> + set ovs-vswitchd >>>> + # DPDK options are expected to be at the beginning of >>>> + # the command line arguments. Add '--' to mark the end >>>> + # of the DPDK options. >>>> + [ -n "$DPDK_OPTS" ] && set -- "$@" $DPDK_OPTS "--" >>>> + set "$@" unix:"$DB_SOCK" >>> >>> NAK - this doesn't work on the latest version of Open vSwitch. >>> >> >> Oh yes right. I only tested that with 2.5.0 but now I see that even the >> 2.5 branch has changed in that regard. > > Oh, this is for 2.5; I'm not sure about it then. Yeah me neither. It's certainly convenient to be able to use /etc/sysconfig/openvswitch to pass all the dpdk options but since that has been fixed in a better way in master perhaps it does not matter match. In any case, I could submit a reworked patch and see what happens. I don't know if there > is such a process to write commits for specific releases, but since 2.5 > is an LTS release it might make sense. As far as I understand, 2.5 will > not backport the dpdk database changes. > > It may acceptable to add something a bit more generic though. I'm > thinking there could be some variables as: > > OVSDB_SERVER_EXTRA > OVS_VSWITCHD_EXTRA Just to make sure we are on the same page, you mean to use these in /etc/sysconfig/openvswitch (much like OPTIONS) and then treat them as environmental variables in ovs-ctl right? because merging them with OPTIONS and passing all of them as arguments is going to be slightly ugly :)
Markos Chandras <mchandras@suse.de> writes: > On 07/05/2016 03:08 PM, Aaron Conole wrote: >> Markos Chandras <mchandras@suse.de> writes: >> >>> On 07/02/2016 03:22 AM, Aaron Conole wrote: >>>> Markos Chandras <mchandras@suse.de> writes: >>>> >>>>> The ovs-ctl script is used to launch ovs-vswitchd among other things. >>>>> However it does not make it possible to pass DPDK options to the >>>>> daemon. We fix this by explicitly looking and extracting the DPDK >>>>> options from the command line which is then reconstructed so it can be >>>>> parsed by ovs-ctl as usual. The DPDK options are later passed to >>>>> ovs-vswitchd. >>>>> >>>>> Signed-off-by: Markos Chandras <mchandras@suse.de> >>>>> --- >>>>> utilities/ovs-ctl.in | 34 ++++++++++++++++++++++++++++++++-- >>>>> 1 file changed, 32 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git a/utilities/ovs-ctl.in b/utilities/ovs-ctl.in >>>>> index 8ec825b..b4e7bf1 100755 >>>>> --- a/utilities/ovs-ctl.in >>>>> +++ b/utilities/ovs-ctl.in >>>>> @@ -230,7 +230,12 @@ do_start_forwarding () { >>>>> fi >>>>> >>>>> # Start ovs-vswitchd. >>>>> - set ovs-vswitchd unix:"$DB_SOCK" >>>>> + set ovs-vswitchd >>>>> + # DPDK options are expected to be at the beginning of >>>>> + # the command line arguments. Add '--' to mark the end >>>>> + # of the DPDK options. >>>>> + [ -n "$DPDK_OPTS" ] && set -- "$@" $DPDK_OPTS "--" >>>>> + set "$@" unix:"$DB_SOCK" >>>> >>>> NAK - this doesn't work on the latest version of Open vSwitch. >>>> >>> >>> Oh yes right. I only tested that with 2.5.0 but now I see that even the >>> 2.5 branch has changed in that regard. >> >> Oh, this is for 2.5; I'm not sure about it then. > > Yeah me neither. It's certainly convenient to be able to use > /etc/sysconfig/openvswitch to pass all the dpdk options but since that > has been fixed in a better way in master perhaps it does not matter > match. In any case, I could submit a reworked patch and see what happens. > > I don't know if there >> is such a process to write commits for specific releases, but since 2.5 >> is an LTS release it might make sense. As far as I understand, 2.5 will >> not backport the dpdk database changes. >> >> It may acceptable to add something a bit more generic though. I'm >> thinking there could be some variables as: >> >> OVSDB_SERVER_EXTRA >> OVS_VSWITCHD_EXTRA > > Just to make sure we are on the same page, you mean to use these in > /etc/sysconfig/openvswitch (much like OPTIONS) and then treat them as > environmental variables in ovs-ctl right? because merging them with > OPTIONS and passing all of them as arguments is going to be slightly ugly :) Well, I haven't thought that far ahead ;-) I was more thinking it would make sense to have some daemon specific variables to pass. However that looks, although I agree adding weird command line switches could be ugly depending on how it was done. I'm just throwing out ideas. I don't promise they'll be any good.
On 07/06/2016 05:26 AM, Aaron Conole wrote: > Markos Chandras <mchandras@suse.de> writes: > >> On 07/05/2016 03:08 PM, Aaron Conole wrote: >>> Markos Chandras <mchandras@suse.de> writes: >>> >>>> On 07/02/2016 03:22 AM, Aaron Conole wrote: >>>>> Markos Chandras <mchandras@suse.de> writes: >>>>> >>>>>> The ovs-ctl script is used to launch ovs-vswitchd among other things. >>>>>> However it does not make it possible to pass DPDK options to the >>>>>> daemon. We fix this by explicitly looking and extracting the DPDK >>>>>> options from the command line which is then reconstructed so it can be >>>>>> parsed by ovs-ctl as usual. The DPDK options are later passed to >>>>>> ovs-vswitchd. >>>>>> >>>>>> Signed-off-by: Markos Chandras <mchandras@suse.de> >>>>>> --- >>>>>> utilities/ovs-ctl.in | 34 ++++++++++++++++++++++++++++++++-- >>>>>> 1 file changed, 32 insertions(+), 2 deletions(-) >>>>>> >>>>>> diff --git a/utilities/ovs-ctl.in b/utilities/ovs-ctl.in >>>>>> index 8ec825b..b4e7bf1 100755 >>>>>> --- a/utilities/ovs-ctl.in >>>>>> +++ b/utilities/ovs-ctl.in >>>>>> @@ -230,7 +230,12 @@ do_start_forwarding () { >>>>>> fi >>>>>> >>>>>> # Start ovs-vswitchd. >>>>>> - set ovs-vswitchd unix:"$DB_SOCK" >>>>>> + set ovs-vswitchd >>>>>> + # DPDK options are expected to be at the beginning of >>>>>> + # the command line arguments. Add '--' to mark the end >>>>>> + # of the DPDK options. >>>>>> + [ -n "$DPDK_OPTS" ] && set -- "$@" $DPDK_OPTS "--" >>>>>> + set "$@" unix:"$DB_SOCK" >>>>> >>>>> NAK - this doesn't work on the latest version of Open vSwitch. >>>>> >>>> >>>> Oh yes right. I only tested that with 2.5.0 but now I see that even the >>>> 2.5 branch has changed in that regard. >>> >>> Oh, this is for 2.5; I'm not sure about it then. >> >> Yeah me neither. It's certainly convenient to be able to use >> /etc/sysconfig/openvswitch to pass all the dpdk options but since that >> has been fixed in a better way in master perhaps it does not matter >> match. In any case, I could submit a reworked patch and see what happens. >> >> I don't know if there >>> is such a process to write commits for specific releases, but since 2.5 >>> is an LTS release it might make sense. As far as I understand, 2.5 will >>> not backport the dpdk database changes. >>> >>> It may acceptable to add something a bit more generic though. I'm >>> thinking there could be some variables as: >>> >>> OVSDB_SERVER_EXTRA >>> OVS_VSWITCHD_EXTRA >> >> Just to make sure we are on the same page, you mean to use these in >> /etc/sysconfig/openvswitch (much like OPTIONS) and then treat them as >> environmental variables in ovs-ctl right? because merging them with >> OPTIONS and passing all of them as arguments is going to be slightly ugly :) > > Well, I haven't thought that far ahead ;-) I was more thinking it would > make sense to have some daemon specific variables to pass. However that > looks, although I agree adding weird command line switches could be > ugly depending on how it was done. I'm just throwing out ideas. I > don't promise they'll be any good. > Yeah it's OK. I will bake a new patch soon.
diff --git a/utilities/ovs-ctl.in b/utilities/ovs-ctl.in index 8ec825b..b4e7bf1 100755 --- a/utilities/ovs-ctl.in +++ b/utilities/ovs-ctl.in @@ -230,7 +230,12 @@ do_start_forwarding () { fi # Start ovs-vswitchd. - set ovs-vswitchd unix:"$DB_SOCK" + set ovs-vswitchd + # DPDK options are expected to be at the beginning of + # the command line arguments. Add '--' to mark the end + # of the DPDK options. + [ -n "$DPDK_OPTS" ] && set -- "$@" $DPDK_OPTS "--" + set "$@" unix:"$DB_SOCK" set "$@" -vconsole:emer -vsyslog:err -vfile:info if test X"$MLOCKALL" != Xno; then set "$@" --mlockall @@ -485,6 +490,25 @@ enable_protocol () { fi } +## ------------------ ## +## parse-dpdk-cmdline ## +## ------------------ ## +parse_dpdk_cmdline () { + echo "${@}" | grep -q -- "--dpdk" || return + + # The DPDK options can be passed anywhere on the command line when + # calling the ovs-ctl script so find out where they are. + if echo "${@}" | grep -q " -- "; then + # end of command line options found + DPDK_OPTS="$(echo ${@} | sed 's/^.*\(--dpdk.*\)\s* -- .*$/\1/')" + cmdline_args="$(echo ${@} | sed 's/\(^.*\)\(--dpdk.*\)\s* -- \(.*$\)/\1\3/')" + else + # no end of command line options found. DPDK options will be at the end + DPDK_OPTS="$(echo ${@} | sed 's/^.*\(--dpdk.*$\)/\1/')" + cmdline_args="$(echo ${@} | sed 's/\(^.*\)\(--dpdk.*$\)/\1/')" + fi +} + ## ---- ## ## main ## ## ---- ## @@ -530,13 +554,15 @@ set_defaults () { SYSTEM_TYPE=unknown SYSTEM_VERSION=unknown fi + + DPDK_OPTS= } usage () { set_defaults cat <<EOF $0: controls Open vSwitch daemons -usage: $0 [OPTIONS] COMMAND +usage: $0 [[OVS_VSWITCHD DPDK OPTIONS] --] [OPTIONS] COMMAND This program is intended to be invoked internally by Open vSwitch startup scripts. System administrators should not normally invoke it directly. @@ -624,7 +650,11 @@ daemons () { set_defaults extra_ids= +cmdline_args="${@}" command= +parse_dpdk_cmdline "${cmdline_args}" +set -- ${cmdline_args} + for arg do case $arg in
The ovs-ctl script is used to launch ovs-vswitchd among other things. However it does not make it possible to pass DPDK options to the daemon. We fix this by explicitly looking and extracting the DPDK options from the command line which is then reconstructed so it can be parsed by ovs-ctl as usual. The DPDK options are later passed to ovs-vswitchd. Signed-off-by: Markos Chandras <mchandras@suse.de> --- utilities/ovs-ctl.in | 34 ++++++++++++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-)