* [PATCH] connman: get the correct network interface name from dmesg during NFS booting @ 2016-10-10 17:35 Jagadeesh Krishnanjanappa 2016-10-11 7:41 ` Andreas Oberritter 0 siblings, 1 reply; 12+ messages in thread From: Jagadeesh Krishnanjanappa @ 2016-10-10 17:35 UTC (permalink / raw) To: openembedded-core Following are the drawbacks with the current logic: 1. If ip=dhcp in the boot command line, then the current code makes connman to ignore "eth0" network interface from managing internet connections. This can cause NFS boot failure, if the network interface used for NFS booting is other than "eth0". 2. If ip=bootp in the boot command line, then none of the network interfaces are ignored. This makes connman to manage internet connections on every active network interfaces (including the network interface used for NFS booting), resulting hang during NFS boot. This patch finds the network interface used for NFS booting via dmesg. It searches for "device=" string from dmesg output and finds name of network interface used for NFS. The "device=" string is printed from dmesg, if IPCONFIG_SILENT macro is disabled in the kernel, and this macro is disabled by default. If "device=" string is not found, then falls back to earlier logic. The earlier logic of detecting NFS network interface is retained, as we cannot determine the exact network interface name used during NFS bootup. Signed-off-by: Jagadeesh Krishnanjanappa <jkrishnanjanappa@mvista.com> --- meta/recipes-connectivity/connman/connman/connman | 35 +++++++++++++---------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/meta/recipes-connectivity/connman/connman/connman b/meta/recipes-connectivity/connman/connman/connman index c64fa0d..aae2ca6 100644 --- a/meta/recipes-connectivity/connman/connman/connman +++ b/meta/recipes-connectivity/connman/connman/connman @@ -29,23 +29,28 @@ done do_start() { EXTRA_PARAM="" if test $nfsroot -eq 1 ; then - NET_DEVS=`cat /proc/net/dev | sed -ne 's/^\([a-zA-Z0-9 ]*\):.*$/\1/p'` - NET_ADDR=`cat /proc/cmdline | sed -ne 's/^.*ip=\([^ :]*\).*$/\1/p'` + ethn_from_dmesg=`dmesg | grep "device="| sed "s|\(.*\)device=\(.*\), hwaddr=\(.*\)|\2|g"` + if [ ! -z "$ethn_from_dmesg" ]; then + EXTRA_PARAM="-I $ethn_from_dmesg" + else + NET_DEVS=`cat /proc/net/dev | sed -ne 's/^\([a-zA-Z0-9 ]*\):.*$/\1/p'` + NET_ADDR=`cat /proc/cmdline | sed -ne 's/^.*ip=\([^ :]*\).*$/\1/p'` - if [ ! -z "$NET_ADDR" ]; then - if [ "$NET_ADDR" = dhcp ]; then - ethn=`ifconfig | grep "^eth" | sed -e "s/\(eth[0-9]\)\(.*\)/\1/"` - if [ ! -z "$ethn" ]; then - EXTRA_PARAM="-I $ethn" - fi - else - for i in $NET_DEVS; do - ADDR=`ifconfig $i | sed 's/addr://g' | sed -ne 's/^.*inet \([0-9.]*\) .*$/\1/p'` - if [ "$NET_ADDR" = "$ADDR" ]; then - EXTRA_PARAM="-I $i" - break + if [ ! -z "$NET_ADDR" ]; then + if [ "$NET_ADDR" = dhcp ]; then + ethn=`ifconfig | grep "^eth" | sed -e "s/\(eth[0-9]\)\(.*\)/\1/"` + if [ ! -z "$ethn" ]; then + EXTRA_PARAM="-I $ethn" fi - done + else + for i in $NET_DEVS; do + ADDR=`ifconfig $i | sed 's/addr://g' | sed -ne 's/^.*inet \([0-9.]*\) .*$/\1/p'` + if [ "$NET_ADDR" = "$ADDR" ]; then + EXTRA_PARAM="-I $i" + break + fi + done + fi fi fi fi -- 2.7.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] connman: get the correct network interface name from dmesg during NFS booting 2016-10-10 17:35 [PATCH] connman: get the correct network interface name from dmesg during NFS booting Jagadeesh Krishnanjanappa @ 2016-10-11 7:41 ` Andreas Oberritter 2016-10-11 12:11 ` Jagadeesh Krishnanjanappa 0 siblings, 1 reply; 12+ messages in thread From: Andreas Oberritter @ 2016-10-11 7:41 UTC (permalink / raw) To: openembedded-core [-- Attachment #1: Type: text/plain, Size: 4017 bytes --] Dear Jagadeesh, On 10.10.2016 19:35, Jagadeesh Krishnanjanappa wrote: > Following are the drawbacks with the current logic: > 1. If ip=dhcp in the boot command line, then the > current code makes connman to ignore "eth0" network interface > from managing internet connections. This can cause NFS boot > failure, if the network interface used for NFS booting > is other than "eth0". > > 2. If ip=bootp in the boot command line, then none of the > network interfaces are ignored. This makes connman to manage > internet connections on every active network interfaces > (including the network interface used for NFS booting), resulting > hang during NFS boot. > > This patch finds the network interface used for NFS booting via dmesg. > It searches for "device=" string from dmesg output and finds > name of network interface used for NFS. The "device=" string > is printed from dmesg, if IPCONFIG_SILENT macro is disabled > in the kernel, and this macro is disabled by default. > > If "device=" string is not found, then falls back to earlier logic. > The earlier logic of detecting NFS network interface is retained, as > we cannot determine the exact network interface name used during > NFS bootup. I think this is too fragile to land in OE-Core. What happens if a network driver prints "device=eth0"? Or if other kernel messages make the string disappear from dmesg and connman gets restarted later? FWIW, I'm attaching my current wrapper script for connmand. I don't think it's perfect, but at least it doesn't depend on any init manager and works across restarts. Add these lines to connman's do_install, in case you'd like to test and/or submit it: mv ${D}${sbindir}/connmand ${D}${sbindir}/connmand.real install -m 755 ${WORKDIR}/connmand-nfsroot.in ${D}${sbindir}/connmand sed -e 's,@sbindir@,${sbindir},g' -i ${D}${sbindir}/connmand Regards, Andreas > > Signed-off-by: Jagadeesh Krishnanjanappa <jkrishnanjanappa@mvista.com> > --- > meta/recipes-connectivity/connman/connman/connman | 35 +++++++++++++---------- > 1 file changed, 20 insertions(+), 15 deletions(-) > > diff --git a/meta/recipes-connectivity/connman/connman/connman b/meta/recipes-connectivity/connman/connman/connman > index c64fa0d..aae2ca6 100644 > --- a/meta/recipes-connectivity/connman/connman/connman > +++ b/meta/recipes-connectivity/connman/connman/connman > @@ -29,23 +29,28 @@ done > do_start() { > EXTRA_PARAM="" > if test $nfsroot -eq 1 ; then > - NET_DEVS=`cat /proc/net/dev | sed -ne 's/^\([a-zA-Z0-9 ]*\):.*$/\1/p'` > - NET_ADDR=`cat /proc/cmdline | sed -ne 's/^.*ip=\([^ :]*\).*$/\1/p'` > + ethn_from_dmesg=`dmesg | grep "device="| sed "s|\(.*\)device=\(.*\), hwaddr=\(.*\)|\2|g"` > + if [ ! -z "$ethn_from_dmesg" ]; then > + EXTRA_PARAM="-I $ethn_from_dmesg" > + else > + NET_DEVS=`cat /proc/net/dev | sed -ne 's/^\([a-zA-Z0-9 ]*\):.*$/\1/p'` > + NET_ADDR=`cat /proc/cmdline | sed -ne 's/^.*ip=\([^ :]*\).*$/\1/p'` > > - if [ ! -z "$NET_ADDR" ]; then > - if [ "$NET_ADDR" = dhcp ]; then > - ethn=`ifconfig | grep "^eth" | sed -e "s/\(eth[0-9]\)\(.*\)/\1/"` > - if [ ! -z "$ethn" ]; then > - EXTRA_PARAM="-I $ethn" > - fi > - else > - for i in $NET_DEVS; do > - ADDR=`ifconfig $i | sed 's/addr://g' | sed -ne 's/^.*inet \([0-9.]*\) .*$/\1/p'` > - if [ "$NET_ADDR" = "$ADDR" ]; then > - EXTRA_PARAM="-I $i" > - break > + if [ ! -z "$NET_ADDR" ]; then > + if [ "$NET_ADDR" = dhcp ]; then > + ethn=`ifconfig | grep "^eth" | sed -e "s/\(eth[0-9]\)\(.*\)/\1/"` > + if [ ! -z "$ethn" ]; then > + EXTRA_PARAM="-I $ethn" > fi > - done > + else > + for i in $NET_DEVS; do > + ADDR=`ifconfig $i | sed 's/addr://g' | sed -ne 's/^.*inet \([0-9.]*\) .*$/\1/p'` > + if [ "$NET_ADDR" = "$ADDR" ]; then > + EXTRA_PARAM="-I $i" > + break > + fi > + done > + fi > fi > fi > fi > [-- Attachment #2: connmand-nfsroot.in --] [-- Type: text/plain, Size: 2391 bytes --] #!/bin/sh # # Copyright (c) 2016 Dream Property GmbH, Germany # http://www.dream-multimedia-tv.de/ # # Permission is hereby granted, free of charge, to any person obtaining a copy # of this software and associated documentation files (the "Software"), to deal # in the Software without restriction, including without limitation the rights # to use, copy, modify, merge, publish, distribute, sublicense, and/or sell # copies of the Software, and to permit persons to whom the Software is # furnished to do so, subject to the following conditions: # # The above copyright notice and this permission notice shall be included in # all copies or substantial portions of the Software. # # THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR # IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, # FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE # AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER # LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN # THE SOFTWARE. parse_cmdline() { local ip= local nfsroot= local root= local options= set -- $(cat /proc/cmdline) for arg do key=${arg%%=*} if [ "$arg" = "$key" ]; then val= else val=${arg#*=} fi case "$key" in ip) ip=${val%%:*} ;; nfsroot) nfsroot=$val ;; root) root=$val ;; esac done if [ "$root" = "/dev/nfs" -o -n "$nfsroot" -a -n "$ip" ]; then set -- $(ip -4 -o addr show | awk '{print $2}') for dev do if ip -o link show $dev | awk '{print $3}' | grep -qvw -e 'LOOPBACK' -e 'NO-CARRIER'; then case "$ip" in any|bootp|both|dhcp|on|rarp) options="-I $dev $options" ;; *) for dev_addr in $(ip -4 -o addr show dev $dev | awk '{print $4}' | grep -o '^[0-9.]\+'); do if [ "$ip" = "$dev_addr" ]; then options="-I $dev" break fi done ;; esac fi done fi echo "$options" } nfsroot_options() { if [ ! -f /run/connman/nfsroot.env ]; then mkdir -p /run/connman parse_cmdline >/run/connman/nfsroot.env fi cat /run/connman/nfsroot.env } exec "@sbindir@/connmand.real" "$@" $(nfsroot_options) ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] connman: get the correct network interface name from dmesg during NFS booting 2016-10-11 7:41 ` Andreas Oberritter @ 2016-10-11 12:11 ` Jagadeesh Krishnanjanappa 2016-10-11 15:36 ` Christopher Larson 2016-10-12 7:00 ` Andreas Oberritter 0 siblings, 2 replies; 12+ messages in thread From: Jagadeesh Krishnanjanappa @ 2016-10-11 12:11 UTC (permalink / raw) To: Andreas Oberritter; +Cc: OE-core [-- Attachment #1: Type: text/plain, Size: 4093 bytes --] Hi Andreas, Thanks for reviewing the patch. > I think this is too fragile to land in OE-Core. What happens if a > network driver prints "device=eth0"? Or if other kernel messages make > the string disappear from dmesg and connman gets restarted later? > True. If device=eth0 gets disappeared/corrupted, then we may have problem. > > FWIW, I'm attaching my current wrapper script for connmand. I don't > think it's perfect, but at least it doesn't depend on any init manager > and works across restarts. The wrapper script attached by you, takes care of the missing scenarios. Seems to be more complete. > > Add these lines to connman's do_install, in case you'd like to test > and/or submit it: > > mv ${D}${sbindir}/connmand ${D}${sbindir}/connmand.real > install -m 755 ${WORKDIR}/connmand-nfsroot.in ${D}${sbindir}/connmand > sed -e 's,@sbindir@,${sbindir},g' -i ${D}${sbindir}/connmand > I think it would be good idea to integrate your changes into the already existing OE-core's connman script, instead of a calling original connman script from the wrapper script. Regards, Jagadeesh > > > Signed-off-by: Jagadeesh Krishnanjanappa <jkrishnanjanappa@mvista.com> > > --- > > meta/recipes-connectivity/connman/connman/connman | 35 > +++++++++++++---------- > > 1 file changed, 20 insertions(+), 15 deletions(-) > > > > diff --git a/meta/recipes-connectivity/connman/connman/connman > b/meta/recipes-connectivity/connman/connman/connman > > index c64fa0d..aae2ca6 100644 > > --- a/meta/recipes-connectivity/connman/connman/connman > > +++ b/meta/recipes-connectivity/connman/connman/connman > > @@ -29,23 +29,28 @@ done > > do_start() { > > EXTRA_PARAM="" > > if test $nfsroot -eq 1 ; then > > - NET_DEVS=`cat /proc/net/dev | sed -ne 's/^\([a-zA-Z0-9 > ]*\):.*$/\1/p'` > > - NET_ADDR=`cat /proc/cmdline | sed -ne 's/^.*ip=\([^ > :]*\).*$/\1/p'` > > + ethn_from_dmesg=`dmesg | grep "device="| sed > "s|\(.*\)device=\(.*\), hwaddr=\(.*\)|\2|g"` > > + if [ ! -z "$ethn_from_dmesg" ]; then > > + EXTRA_PARAM="-I $ethn_from_dmesg" > > + else > > + NET_DEVS=`cat /proc/net/dev | sed -ne 's/^\([a-zA-Z0-9 > ]*\):.*$/\1/p'` > > + NET_ADDR=`cat /proc/cmdline | sed -ne 's/^.*ip=\([^ > :]*\).*$/\1/p'` > > > > - if [ ! -z "$NET_ADDR" ]; then > > - if [ "$NET_ADDR" = dhcp ]; then > > - ethn=`ifconfig | grep "^eth" | sed -e > "s/\(eth[0-9]\)\(.*\)/\1/"` > > - if [ ! -z "$ethn" ]; then > > - EXTRA_PARAM="-I $ethn" > > - fi > > - else > > - for i in $NET_DEVS; do > > - ADDR=`ifconfig $i | sed 's/addr://g' | sed -ne > 's/^.*inet \([0-9.]*\) .*$/\1/p'` > > - if [ "$NET_ADDR" = "$ADDR" ]; then > > - EXTRA_PARAM="-I $i" > > - break > > + if [ ! -z "$NET_ADDR" ]; then > > + if [ "$NET_ADDR" = dhcp ]; then > > + ethn=`ifconfig | grep "^eth" | sed -e > "s/\(eth[0-9]\)\(.*\)/\1/"` > > + if [ ! -z "$ethn" ]; then > > + EXTRA_PARAM="-I $ethn" > > fi > > - done > > + else > > + for i in $NET_DEVS; do > > + ADDR=`ifconfig $i | sed 's/addr://g' | sed -ne > 's/^.*inet \([0-9.]*\) .*$/\1/p'` > > + if [ "$NET_ADDR" = "$ADDR" ]; then > > + EXTRA_PARAM="-I $i" > > + break > > + fi > > + done > > + fi > > fi > > fi > > fi > > > > > -- > _______________________________________________ > Openembedded-core mailing list > Openembedded-core@lists.openembedded.org > http://lists.openembedded.org/mailman/listinfo/openembedded-core > > [-- Attachment #2: Type: text/html, Size: 6586 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] connman: get the correct network interface name from dmesg during NFS booting 2016-10-11 12:11 ` Jagadeesh Krishnanjanappa @ 2016-10-11 15:36 ` Christopher Larson 2016-10-11 19:03 ` Jagadeesh Krishnanjanappa 2016-10-12 7:00 ` Andreas Oberritter 1 sibling, 1 reply; 12+ messages in thread From: Christopher Larson @ 2016-10-11 15:36 UTC (permalink / raw) To: Jagadeesh Krishnanjanappa; +Cc: OE-core [-- Attachment #1: Type: text/plain, Size: 1676 bytes --] On Tue, Oct 11, 2016 at 5:11 AM, Jagadeesh Krishnanjanappa < jkrishnanjanappa@mvista.com> wrote: > Thanks for reviewing the patch. > > >> I think this is too fragile to land in OE-Core. What happens if a >> network driver prints "device=eth0"? Or if other kernel messages make >> the string disappear from dmesg and connman gets restarted later? >> > > True. If device=eth0 gets disappeared/corrupted, then we may have problem. > >> >> FWIW, I'm attaching my current wrapper script for connmand. I don't >> think it's perfect, but at least it doesn't depend on any init manager >> and works across restarts. > > The wrapper script attached by you, takes care of the missing scenarios. > Seems to be more complete. > >> >> > Add these lines to connman's do_install, in case you'd like to test >> and/or submit it: >> >> mv ${D}${sbindir}/connmand ${D}${sbindir}/connmand.real >> install -m 755 ${WORKDIR}/connmand-nfsroot.in ${D}${sbindir}/connmand >> sed -e 's,@sbindir@,${sbindir},g' -i ${D}${sbindir}/connmand >> > I think it would be good idea to integrate your changes into the already > existing OE-core's connman script, instead of a calling original connman > script from the wrapper script. > I threw together https://github.com/openembedded/openembedded-core/compare/master...kergoth:connman-systemd-nfs last night, only limited testing. I think using a script in libexecdir is rather cleaner than wrapping connmand in place, for something like this, personally. -- Christopher Larson clarson at kergoth dot com Founder - BitBake, OpenEmbedded, OpenZaurus Maintainer - Tslib Senior Software Engineer, Mentor Graphics [-- Attachment #2: Type: text/html, Size: 3238 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] connman: get the correct network interface name from dmesg during NFS booting 2016-10-11 15:36 ` Christopher Larson @ 2016-10-11 19:03 ` Jagadeesh Krishnanjanappa 2016-10-11 19:34 ` Christopher Larson 0 siblings, 1 reply; 12+ messages in thread From: Jagadeesh Krishnanjanappa @ 2016-10-11 19:03 UTC (permalink / raw) To: Christopher Larson; +Cc: OE-core [-- Attachment #1: Type: text/plain, Size: 2061 bytes --] On Tue, Oct 11, 2016 at 9:06 PM, Christopher Larson <clarson@kergoth.com> wrote: > > On Tue, Oct 11, 2016 at 5:11 AM, Jagadeesh Krishnanjanappa < > jkrishnanjanappa@mvista.com> wrote: > >> Thanks for reviewing the patch. >> >> >>> I think this is too fragile to land in OE-Core. What happens if a >>> network driver prints "device=eth0"? Or if other kernel messages make >>> the string disappear from dmesg and connman gets restarted later? >>> >> >> True. If device=eth0 gets disappeared/corrupted, then we may have problem. >> >>> >>> FWIW, I'm attaching my current wrapper script for connmand. I don't >>> think it's perfect, but at least it doesn't depend on any init manager >>> and works across restarts. >> >> The wrapper script attached by you, takes care of the missing scenarios. >> Seems to be more complete. >> >>> >>> >> Add these lines to connman's do_install, in case you'd like to test >>> and/or submit it: >>> >>> mv ${D}${sbindir}/connmand ${D}${sbindir}/connmand.real >>> install -m 755 ${WORKDIR}/connmand-nfsroot.in ${D}${sbindir}/connmand >>> sed -e 's,@sbindir@,${sbindir},g' -i ${D}${sbindir}/connmand >>> >> I think it would be good idea to integrate your changes into the already >> existing OE-core's connman script, instead of a calling original connman >> script from the wrapper script. >> > > I threw together https://github.com/openembedded/openembedded- > core/compare/master...kergoth:connman-systemd-nfs last night, > only limited testing. I think using a script in libexecdir is rather > cleaner than wrapping connmand in place, for something like this, > personally. > In the above commit, the last line of connman/connman/start-connman file, should be exec @SBINDIR@/connmand "$@" instead of exec @SBINDIR@/connman "$@" ; as the actual binary file name is connmand. Regards, Jagadeesh > > > -- > Christopher Larson > clarson at kergoth dot com > Founder - BitBake, OpenEmbedded, OpenZaurus > Maintainer - Tslib > Senior Software Engineer, Mentor Graphics > [-- Attachment #2: Type: text/html, Size: 4432 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] connman: get the correct network interface name from dmesg during NFS booting 2016-10-11 19:03 ` Jagadeesh Krishnanjanappa @ 2016-10-11 19:34 ` Christopher Larson 2016-10-12 7:30 ` Andreas Oberritter 0 siblings, 1 reply; 12+ messages in thread From: Christopher Larson @ 2016-10-11 19:34 UTC (permalink / raw) To: Jagadeesh Krishnanjanappa; +Cc: OE-core [-- Attachment #1: Type: text/plain, Size: 2326 bytes --] On Tue, Oct 11, 2016 at 12:03 PM, Jagadeesh Krishnanjanappa < jkrishnanjanappa@mvista.com> wrote: > On Tue, Oct 11, 2016 at 9:06 PM, Christopher Larson <clarson@kergoth.com> > wrote: > >> >> On Tue, Oct 11, 2016 at 5:11 AM, Jagadeesh Krishnanjanappa < >> jkrishnanjanappa@mvista.com> wrote: >> >>> Thanks for reviewing the patch. >>> >>> >>>> I think this is too fragile to land in OE-Core. What happens if a >>>> network driver prints "device=eth0"? Or if other kernel messages make >>>> the string disappear from dmesg and connman gets restarted later? >>>> >>> >>> True. If device=eth0 gets disappeared/corrupted, then we may have >>> problem. >>> >>>> >>>> FWIW, I'm attaching my current wrapper script for connmand. I don't >>>> think it's perfect, but at least it doesn't depend on any init manager >>>> and works across restarts. >>> >>> The wrapper script attached by you, takes care of the missing scenarios. >>> Seems to be more complete. >>> >>>> >>>> >>> Add these lines to connman's do_install, in case you'd like to test >>>> and/or submit it: >>>> >>>> mv ${D}${sbindir}/connmand ${D}${sbindir}/connmand.real >>>> install -m 755 ${WORKDIR}/connmand-nfsroot.in ${D}${sbindir}/connmand >>>> sed -e 's,@sbindir@,${sbindir},g' -i ${D}${sbindir}/connmand >>>> >>> I think it would be good idea to integrate your changes into the already >>> existing OE-core's connman script, instead of a calling original connman >>> script from the wrapper script. >>> >> >> I threw together https://github.com/openembedded/openembedded-core/ >> compare/master...kergoth:connman-systemd-nfs last night, only limited >> testing. I think using a script in libexecdir is rather cleaner than >> wrapping connmand in place, for something like this, personally. >> > > In the above commit, the last line of connman/connman/start-connman file, > should be exec @SBINDIR@/connmand "$@" instead of exec @SBINDIR@/connman > "$@" ; as the actual binary file name is connmand. Thanks, didn’t notice that. Obviously it needs polish and testing before submission, I mainly linked it to solicit thoughts on the approach :) -- Christopher Larson clarson at kergoth dot com Founder - BitBake, OpenEmbedded, OpenZaurus Maintainer - Tslib Senior Software Engineer, Mentor Graphics [-- Attachment #2: Type: text/html, Size: 4691 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] connman: get the correct network interface name from dmesg during NFS booting 2016-10-11 19:34 ` Christopher Larson @ 2016-10-12 7:30 ` Andreas Oberritter 2016-10-12 17:49 ` Christopher Larson 0 siblings, 1 reply; 12+ messages in thread From: Andreas Oberritter @ 2016-10-12 7:30 UTC (permalink / raw) To: Christopher Larson, Jagadeesh Krishnanjanappa; +Cc: OE-core Hi Chistopher, On 11.10.2016 21:34, Christopher Larson wrote: > > On Tue, Oct 11, 2016 at 12:03 PM, Jagadeesh Krishnanjanappa > <jkrishnanjanappa@mvista.com <mailto:jkrishnanjanappa@mvista.com>> wrote: > > On Tue, Oct 11, 2016 at 9:06 PM, Christopher Larson > <clarson@kergoth.com <mailto:clarson@kergoth.com>> wrote: > > > On Tue, Oct 11, 2016 at 5:11 AM, Jagadeesh Krishnanjanappa > <jkrishnanjanappa@mvista.com > <mailto:jkrishnanjanappa@mvista.com>> wrote: > > Thanks for reviewing the patch. > > > I think this is too fragile to land in OE-Core. What > happens if a > network driver prints "device=eth0"? Or if other kernel > messages make > the string disappear from dmesg and connman gets > restarted later? > > > True. If device=eth0 gets disappeared/corrupted, then we may > have problem. > > > FWIW, I'm attaching my current wrapper script for > connmand. I don't > think it's perfect, but at least it doesn't depend on > any init manager > and works across restarts. > > The wrapper script attached by you, takes care of the > missing scenarios. Seems to be more complete. > > > > Add these lines to connman's do_install, in case you'd > like to test > and/or submit it: > > mv ${D}${sbindir}/connmand ${D}${sbindir}/connmand.real > install -m 755 ${WORKDIR}/connmand-nfsroot.in > <http://connmand-nfsroot.in> ${D}${sbindir}/connmand > sed -e 's,@sbindir@,${sbindir},g' -i ${D}${sbindir}/connmand > > I think it would be good idea to integrate your changes into > the already existing OE-core's connman script, instead of a > calling original connman script from the wrapper script. > > > I threw > together https://github.com/openembedded/openembedded-core/compare/master...kergoth:connman-systemd-nfs > <https://github.com/openembedded/openembedded-core/compare/master...kergoth:connman-systemd-nfs> last > night, only limited testing. I think using a script in > libexecdir is rather cleaner than wrapping connmand in place, > for something like this, personally. > > > In the above commit, the last line of connman/connman/start-connman > file, should be exec @SBINDIR@/connmand "$@" instead of exec > @SBINDIR@/connman "$@" ; as the actual binary file name is connmand. > > > Thanks, didn’t notice that. Obviously it needs polish and testing before > submission, I mainly linked it to solicit thoughts on the approach :) your proposal adds some complexity to the recipe, which I tried to avoid, because this wrapper script is something we'll probably have to maintain forever. For example, your recipe might break without notice if upstream is going to ship another binary as ${sbindir}/connmand-something in the future; or if upstream decides to change the systemd unit to use a variable instead of a hardcoded path to connmand. Besides that, I don't have a strong opinion on the location of the wrapper script. Regards, Andreas ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] connman: get the correct network interface name from dmesg during NFS booting 2016-10-12 7:30 ` Andreas Oberritter @ 2016-10-12 17:49 ` Christopher Larson 2016-10-12 19:08 ` Andreas Oberritter 0 siblings, 1 reply; 12+ messages in thread From: Christopher Larson @ 2016-10-12 17:49 UTC (permalink / raw) To: Andreas Oberritter; +Cc: OE-core [-- Attachment #1: Type: text/plain, Size: 4099 bytes --] On Wed, Oct 12, 2016 at 12:30 AM, Andreas Oberritter <obi@opendreambox.org> wrote: > On 11.10.2016 21:34, Christopher Larson wrote: > > > > On Tue, Oct 11, 2016 at 12:03 PM, Jagadeesh Krishnanjanappa > > <jkrishnanjanappa@mvista.com <mailto:jkrishnanjanappa@mvista.com>> > wrote: > > > > On Tue, Oct 11, 2016 at 9:06 PM, Christopher Larson > > <clarson@kergoth.com <mailto:clarson@kergoth.com>> wrote: > > > > > > On Tue, Oct 11, 2016 at 5:11 AM, Jagadeesh Krishnanjanappa > > <jkrishnanjanappa@mvista.com > > <mailto:jkrishnanjanappa@mvista.com>> wrote: > > > > Thanks for reviewing the patch. > > > > > > I think this is too fragile to land in OE-Core. What > > happens if a > > network driver prints "device=eth0"? Or if other kernel > > messages make > > the string disappear from dmesg and connman gets > > restarted later? > > > > > > True. If device=eth0 gets disappeared/corrupted, then we may > > have problem. > > > > > > FWIW, I'm attaching my current wrapper script for > > connmand. I don't > > think it's perfect, but at least it doesn't depend on > > any init manager > > and works across restarts. > > > > The wrapper script attached by you, takes care of the > > missing scenarios. Seems to be more complete. > > > > > > > > Add these lines to connman's do_install, in case you'd > > like to test > > and/or submit it: > > > > mv ${D}${sbindir}/connmand ${D}${sbindir}/connmand.real > > install -m 755 ${WORKDIR}/connmand-nfsroot.in > > <http://connmand-nfsroot.in> ${D}${sbindir}/connmand > > sed -e 's,@sbindir@,${sbindir},g' -i > ${D}${sbindir}/connmand > > > > I think it would be good idea to integrate your changes into > > the already existing OE-core's connman script, instead of a > > calling original connman script from the wrapper script. > > > > > > I threw > > together https://github.com/openembedde > d/openembedded-core/compare/master...kergoth:connman-systemd-nfs > > <https://github.com/openembedded/openembedded-core/compare/ > master...kergoth:connman-systemd-nfs> last > > night, only limited testing. I think using a script in > > libexecdir is rather cleaner than wrapping connmand in place, > > for something like this, personally. > > > > > > In the above commit, the last line of connman/connman/start-connman > > file, should be exec @SBINDIR@/connmand "$@" instead of exec > > @SBINDIR@/connman "$@" ; as the actual binary file name is connmand. > > > > > > Thanks, didn’t notice that. Obviously it needs polish and testing before > > submission, I mainly linked it to solicit thoughts on the approach :) > > your proposal adds some complexity to the recipe, which I tried to > avoid, because this wrapper script is something we'll probably have to > maintain forever. > > For example, your recipe might break without notice if upstream is going > to ship another binary as ${sbindir}/connmand-something in the future; > or if upstream decides to change the systemd unit to use a variable > instead of a hardcoded path to connmand. > > Besides that, I don't have a strong opinion on the location of the > wrapper script. Good points. I think the future-proofing could be addressed with something like sed -i -e "s#^ExecStart=.*#ExecStart=$wrapperpath#" ${D}${systemd_unitdir}/system/connman.service, and then switch away from that weak start-connman script in favor of yours, but keep it in libexec for cleanliness. -- Christopher Larson clarson at kergoth dot com Founder - BitBake, OpenEmbedded, OpenZaurus Maintainer - Tslib Senior Software Engineer, Mentor Graphics [-- Attachment #2: Type: text/html, Size: 6420 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] connman: get the correct network interface name from dmesg during NFS booting 2016-10-12 17:49 ` Christopher Larson @ 2016-10-12 19:08 ` Andreas Oberritter 0 siblings, 0 replies; 12+ messages in thread From: Andreas Oberritter @ 2016-10-12 19:08 UTC (permalink / raw) To: Christopher Larson; +Cc: OE-core On 12.10.2016 19:49, Christopher Larson wrote: > > On Wed, Oct 12, 2016 at 12:30 AM, Andreas Oberritter > <obi@opendreambox.org <mailto:obi@opendreambox.org>> wrote: > > On 11.10.2016 21:34, Christopher Larson wrote: > > > > On Tue, Oct 11, 2016 at 12:03 PM, Jagadeesh Krishnanjanappa > > <jkrishnanjanappa@mvista.com <mailto:jkrishnanjanappa@mvista.com> > <mailto:jkrishnanjanappa@mvista.com > <mailto:jkrishnanjanappa@mvista.com>>> wrote: > > > > On Tue, Oct 11, 2016 at 9:06 PM, Christopher Larson > > <clarson@kergoth.com <mailto:clarson@kergoth.com> > <mailto:clarson@kergoth.com <mailto:clarson@kergoth.com>>> wrote: > > > > > > On Tue, Oct 11, 2016 at 5:11 AM, Jagadeesh Krishnanjanappa > > <jkrishnanjanappa@mvista.com <mailto:jkrishnanjanappa@mvista.com> > > <mailto:jkrishnanjanappa@mvista.com <mailto:jkrishnanjanappa@mvista.com>>> > wrote: > > > > Thanks for reviewing the patch. > > > > > > I think this is too fragile to land in OE-Core. What > > happens if a > > network driver prints "device=eth0"? Or if other kernel > > messages make > > the string disappear from dmesg and connman gets > > restarted later? > > > > > > True. If device=eth0 gets disappeared/corrupted, then we may > > have problem. > > > > > > FWIW, I'm attaching my current wrapper script for > > connmand. I don't > > think it's perfect, but at least it doesn't depend on > > any init manager > > and works across restarts. > > > > The wrapper script attached by you, takes care of the > > missing scenarios. Seems to be more complete. > > > > > > > > Add these lines to connman's do_install, in case you'd > > like to test > > and/or submit it: > > > > mv ${D}${sbindir}/connmand ${D}${sbindir}/connmand.real > > install -m 755 ${WORKDIR}/connmand-nfsroot.in <http://connmand-nfsroot.in> > > <http://connmand-nfsroot.in> ${D}${sbindir}/connmand > > sed -e 's,@sbindir@,${sbindir},g' -i ${D}${sbindir}/connmand > > > > I think it would be good idea to integrate your changes into > > the already existing OE-core's connman script, instead of a > > calling original connman script from the wrapper script. > > > > > > I threw > > together https://github.com/openembedded/openembedded-core/compare/master...kergoth:connman-systemd-nfs > <https://github.com/openembedded/openembedded-core/compare/master...kergoth:connman-systemd-nfs> > > <https://github.com/openembedded/openembedded-core/compare/master...kergoth:connman-systemd-nfs > <https://github.com/openembedded/openembedded-core/compare/master...kergoth:connman-systemd-nfs>> > last > > night, only limited testing. I think using a script in > > libexecdir is rather cleaner than wrapping connmand in place, > > for something like this, personally. > > > > > > In the above commit, the last line of connman/connman/start-connman > > file, should be exec @SBINDIR@/connmand "$@" instead of exec > > @SBINDIR@/connman "$@" ; as the actual binary file name is connmand. > > > > > > Thanks, didn’t notice that. Obviously it needs polish and testing before > > submission, I mainly linked it to solicit thoughts on the approach :) > > your proposal adds some complexity to the recipe, which I tried to > avoid, because this wrapper script is something we'll probably have to > maintain forever. > > For example, your recipe might break without notice if upstream is going > to ship another binary as ${sbindir}/connmand-something in the future; > or if upstream decides to change the systemd unit to use a variable > instead of a hardcoded path to connmand. > > Besides that, I don't have a strong opinion on the location of the > wrapper script. > > > Good points. I think the future-proofing could be addressed with > something like sed -i -e "s#^ExecStart=.*#ExecStart=$wrapperpath#" > ${D}${systemd_unitdir}/system/connman.service, and then switch away from > that weak start-connman script in favor of yours, but keep it in libexec > for cleanliness. Agreed. Regards, Andreas ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] connman: get the correct network interface name from dmesg during NFS booting 2016-10-11 12:11 ` Jagadeesh Krishnanjanappa 2016-10-11 15:36 ` Christopher Larson @ 2016-10-12 7:00 ` Andreas Oberritter 2016-10-13 2:01 ` Jagadeesh Krishnanjanappa 1 sibling, 1 reply; 12+ messages in thread From: Andreas Oberritter @ 2016-10-12 7:00 UTC (permalink / raw) To: Jagadeesh Krishnanjanappa; +Cc: OE-core Hi Jagadeesh, On 11.10.2016 14:11, Jagadeesh Krishnanjanappa wrote: > Hi Andreas, > > Thanks for reviewing the patch. > > > I think this is too fragile to land in OE-Core. What happens if a > network driver prints "device=eth0"? Or if other kernel messages make > the string disappear from dmesg and connman gets restarted later? > > > True. If device=eth0 gets disappeared/corrupted, then we may have problem. > > > FWIW, I'm attaching my current wrapper script for connmand. I don't > think it's perfect, but at least it doesn't depend on any init manager > and works across restarts. > > The wrapper script attached by you, takes care of the missing scenarios. > Seems to be more complete. > > > > Add these lines to connman's do_install, in case you'd like to test > and/or submit it: > > mv ${D}${sbindir}/connmand ${D}${sbindir}/connmand.real > install -m 755 ${WORKDIR}/connmand-nfsroot.in > <http://connmand-nfsroot.in> ${D}${sbindir}/connmand > sed -e 's,@sbindir@,${sbindir},g' -i ${D}${sbindir}/connmand > > I think it would be good idea to integrate your changes into the already > existing OE-core's connman script, instead of a calling original connman > script from the wrapper script. please keep in mind that you'd need to implement the same logic for systemd. Considering that the execution of connman on nfsroots without appropriate parameters immediately hangs the system, a wrapper script offers an additional level of protection compared to an init script or systemd unit, especially for people debugging connman. Regards, Andreas > > Regards, > Jagadeesh > > > > > Signed-off-by: Jagadeesh Krishnanjanappa > <jkrishnanjanappa@mvista.com <mailto:jkrishnanjanappa@mvista.com>> > > --- > > meta/recipes-connectivity/connman/connman/connman | 35 > +++++++++++++---------- > > 1 file changed, 20 insertions(+), 15 deletions(-) > > > > diff --git a/meta/recipes-connectivity/connman/connman/connman > b/meta/recipes-connectivity/connman/connman/connman > > index c64fa0d..aae2ca6 100644 > > --- a/meta/recipes-connectivity/connman/connman/connman > > +++ b/meta/recipes-connectivity/connman/connman/connman > > @@ -29,23 +29,28 @@ done > > do_start() { > > EXTRA_PARAM="" > > if test $nfsroot -eq 1 ; then > > - NET_DEVS=`cat /proc/net/dev | sed -ne 's/^\([a-zA-Z0-9 > ]*\):.*$/\1/p'` > > - NET_ADDR=`cat /proc/cmdline | sed -ne 's/^.*ip=\([^ > :]*\).*$/\1/p'` > > + ethn_from_dmesg=`dmesg | grep "device="| sed > "s|\(.*\)device=\(.*\), hwaddr=\(.*\)|\2|g"` > > + if [ ! -z "$ethn_from_dmesg" ]; then > > + EXTRA_PARAM="-I $ethn_from_dmesg" > > + else > > + NET_DEVS=`cat /proc/net/dev | sed -ne > 's/^\([a-zA-Z0-9 ]*\):.*$/\1/p'` > > + NET_ADDR=`cat /proc/cmdline | sed -ne 's/^.*ip=\([^ > :]*\).*$/\1/p'` > > > > - if [ ! -z "$NET_ADDR" ]; then > > - if [ "$NET_ADDR" = dhcp ]; then > > - ethn=`ifconfig | grep "^eth" | sed -e > "s/\(eth[0-9]\)\(.*\)/\1/"` > > - if [ ! -z "$ethn" ]; then > > - EXTRA_PARAM="-I $ethn" > > - fi > > - else > > - for i in $NET_DEVS; do > > - ADDR=`ifconfig $i | sed 's/addr://g' | sed > -ne 's/^.*inet \([0-9.]*\) .*$/\1/p'` > > - if [ "$NET_ADDR" = "$ADDR" ]; then > > - EXTRA_PARAM="-I $i" > > - break > > + if [ ! -z "$NET_ADDR" ]; then > > + if [ "$NET_ADDR" = dhcp ]; then > > + ethn=`ifconfig | grep "^eth" | sed -e > "s/\(eth[0-9]\)\(.*\)/\1/"` > > + if [ ! -z "$ethn" ]; then > > + EXTRA_PARAM="-I $ethn" > > fi > > - done > > + else > > + for i in $NET_DEVS; do > > + ADDR=`ifconfig $i | sed 's/addr://g' | > sed -ne 's/^.*inet \([0-9.]*\) .*$/\1/p'` > > + if [ "$NET_ADDR" = "$ADDR" ]; then > > + EXTRA_PARAM="-I $i" > > + break > > + fi > > + done > > + fi > > fi > > fi > > fi > > > > > -- > _______________________________________________ > Openembedded-core mailing list > Openembedded-core@lists.openembedded.org > <mailto:Openembedded-core@lists.openembedded.org> > http://lists.openembedded.org/mailman/listinfo/openembedded-core > <http://lists.openembedded.org/mailman/listinfo/openembedded-core> > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] connman: get the correct network interface name from dmesg during NFS booting 2016-10-12 7:00 ` Andreas Oberritter @ 2016-10-13 2:01 ` Jagadeesh Krishnanjanappa 2016-10-19 18:52 ` Jagadeesh Krishnanjanappa 0 siblings, 1 reply; 12+ messages in thread From: Jagadeesh Krishnanjanappa @ 2016-10-13 2:01 UTC (permalink / raw) To: Andreas Oberritter; +Cc: OE-core [-- Attachment #1: Type: text/plain, Size: 5680 bytes --] On Wed, Oct 12, 2016 at 12:30 PM, Andreas Oberritter <obi@opendreambox.org> wrote: > Hi Jagadeesh, > > On 11.10.2016 14:11, Jagadeesh Krishnanjanappa wrote: > > Hi Andreas, > > > > Thanks for reviewing the patch. > > > > > > I think this is too fragile to land in OE-Core. What happens if a > > network driver prints "device=eth0"? Or if other kernel messages make > > the string disappear from dmesg and connman gets restarted later? > > > > > > True. If device=eth0 gets disappeared/corrupted, then we may have > problem. > > > > > > FWIW, I'm attaching my current wrapper script for connmand. I don't > > think it's perfect, but at least it doesn't depend on any init > manager > > and works across restarts. > > > > The wrapper script attached by you, takes care of the missing scenarios. > > Seems to be more complete. > > > > > > > > Add these lines to connman's do_install, in case you'd like to test > > and/or submit it: > > > > mv ${D}${sbindir}/connmand ${D}${sbindir}/connmand.real > > install -m 755 ${WORKDIR}/connmand-nfsroot.in > > <http://connmand-nfsroot.in> ${D}${sbindir}/connmand > > sed -e 's,@sbindir@,${sbindir},g' -i ${D}${sbindir}/connmand > > > > I think it would be good idea to integrate your changes into the already > > existing OE-core's connman script, instead of a calling original connman > > script from the wrapper script. > > please keep in mind that you'd need to implement the same logic for > systemd. > > Considering that the execution of connman on nfsroots without > appropriate parameters immediately hangs the system, a wrapper script > offers an additional level of protection compared to an init script or > systemd unit, especially for people debugging connman. > > The intention of integrating your changes into init script was to avoid an extra file being added to the layer. But yes, to help debugging and to support systemd; wrapper script can be helpful. Thanks, Jagadeesh Regards, > Andreas > > > > > Regards, > > Jagadeesh > > > > > > > > Signed-off-by: Jagadeesh Krishnanjanappa > > <jkrishnanjanappa@mvista.com <mailto:jkrishnanjanappa@mvista.com>> > > > --- > > > meta/recipes-connectivity/connman/connman/connman | 35 > > +++++++++++++---------- > > > 1 file changed, 20 insertions(+), 15 deletions(-) > > > > > > diff --git a/meta/recipes-connectivity/connman/connman/connman > > b/meta/recipes-connectivity/connman/connman/connman > > > index c64fa0d..aae2ca6 100644 > > > --- a/meta/recipes-connectivity/connman/connman/connman > > > +++ b/meta/recipes-connectivity/connman/connman/connman > > > @@ -29,23 +29,28 @@ done > > > do_start() { > > > EXTRA_PARAM="" > > > if test $nfsroot -eq 1 ; then > > > - NET_DEVS=`cat /proc/net/dev | sed -ne 's/^\([a-zA-Z0-9 > > ]*\):.*$/\1/p'` > > > - NET_ADDR=`cat /proc/cmdline | sed -ne 's/^.*ip=\([^ > > :]*\).*$/\1/p'` > > > + ethn_from_dmesg=`dmesg | grep "device="| sed > > "s|\(.*\)device=\(.*\), hwaddr=\(.*\)|\2|g"` > > > + if [ ! -z "$ethn_from_dmesg" ]; then > > > + EXTRA_PARAM="-I $ethn_from_dmesg" > > > + else > > > + NET_DEVS=`cat /proc/net/dev | sed -ne > > 's/^\([a-zA-Z0-9 ]*\):.*$/\1/p'` > > > + NET_ADDR=`cat /proc/cmdline | sed -ne 's/^.*ip=\([^ > > :]*\).*$/\1/p'` > > > > > > - if [ ! -z "$NET_ADDR" ]; then > > > - if [ "$NET_ADDR" = dhcp ]; then > > > - ethn=`ifconfig | grep "^eth" | sed -e > > "s/\(eth[0-9]\)\(.*\)/\1/"` > > > - if [ ! -z "$ethn" ]; then > > > - EXTRA_PARAM="-I $ethn" > > > - fi > > > - else > > > - for i in $NET_DEVS; do > > > - ADDR=`ifconfig $i | sed 's/addr://g' | sed > > -ne 's/^.*inet \([0-9.]*\) .*$/\1/p'` > > > - if [ "$NET_ADDR" = "$ADDR" ]; then > > > - EXTRA_PARAM="-I $i" > > > - break > > > + if [ ! -z "$NET_ADDR" ]; then > > > + if [ "$NET_ADDR" = dhcp ]; then > > > + ethn=`ifconfig | grep "^eth" | sed -e > > "s/\(eth[0-9]\)\(.*\)/\1/"` > > > + if [ ! -z "$ethn" ]; then > > > + EXTRA_PARAM="-I $ethn" > > > fi > > > - done > > > + else > > > + for i in $NET_DEVS; do > > > + ADDR=`ifconfig $i | sed 's/addr://g' | > > sed -ne 's/^.*inet \([0-9.]*\) .*$/\1/p'` > > > + if [ "$NET_ADDR" = "$ADDR" ]; then > > > + EXTRA_PARAM="-I $i" > > > + break > > > + fi > > > + done > > > + fi > > > fi > > > fi > > > fi > > > > > > > > > -- > > _______________________________________________ > > Openembedded-core mailing list > > Openembedded-core@lists.openembedded.org > > <mailto:Openembedded-core@lists.openembedded.org> > > http://lists.openembedded.org/mailman/listinfo/openembedded-core > > <http://lists.openembedded.org/mailman/listinfo/openembedded-core> > > > > > > [-- Attachment #2: Type: text/html, Size: 8668 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] connman: get the correct network interface name from dmesg during NFS booting 2016-10-13 2:01 ` Jagadeesh Krishnanjanappa @ 2016-10-19 18:52 ` Jagadeesh Krishnanjanappa 0 siblings, 0 replies; 12+ messages in thread From: Jagadeesh Krishnanjanappa @ 2016-10-19 18:52 UTC (permalink / raw) To: Andreas Oberritter, Christopher Larson; +Cc: OE-core [-- Attachment #1: Type: text/plain, Size: 6200 bytes --] Hi Christopher, Thanks for applying the required changes under below commit, https://github.com/openembedded/openembedded-core/compare/master...kergoth:connman-systemd-nfs Are there any plans to submit it to OE-core? Regards, Jagadeesh On Thu, Oct 13, 2016 at 7:31 AM, Jagadeesh Krishnanjanappa < jkrishnanjanappa@mvista.com> wrote: > > > On Wed, Oct 12, 2016 at 12:30 PM, Andreas Oberritter <obi@opendreambox.org > > wrote: > >> Hi Jagadeesh, >> >> On 11.10.2016 14:11, Jagadeesh Krishnanjanappa wrote: >> > Hi Andreas, >> > >> > Thanks for reviewing the patch. >> > >> > >> > I think this is too fragile to land in OE-Core. What happens if a >> > network driver prints "device=eth0"? Or if other kernel messages >> make >> > the string disappear from dmesg and connman gets restarted later? >> > >> > >> > True. If device=eth0 gets disappeared/corrupted, then we may have >> problem. >> > >> > >> > FWIW, I'm attaching my current wrapper script for connmand. I don't >> > think it's perfect, but at least it doesn't depend on any init >> manager >> > and works across restarts. >> > >> > The wrapper script attached by you, takes care of the missing scenarios. >> > Seems to be more complete. >> > >> > >> > >> > Add these lines to connman's do_install, in case you'd like to test >> > and/or submit it: >> > >> > mv ${D}${sbindir}/connmand ${D}${sbindir}/connmand.real >> > install -m 755 ${WORKDIR}/connmand-nfsroot.in >> > <http://connmand-nfsroot.in> ${D}${sbindir}/connmand >> > sed -e 's,@sbindir@,${sbindir},g' -i ${D}${sbindir}/connmand >> > >> > I think it would be good idea to integrate your changes into the already >> > existing OE-core's connman script, instead of a calling original connman >> > script from the wrapper script. >> >> please keep in mind that you'd need to implement the same logic for >> systemd. >> >> Considering that the execution of connman on nfsroots without >> appropriate parameters immediately hangs the system, a wrapper script >> offers an additional level of protection compared to an init script or >> systemd unit, especially for people debugging connman. >> >> The intention of integrating your changes into init script was to avoid > an extra file being added to the layer. But yes, to help debugging and to > support systemd; wrapper script can be helpful. > > Thanks, > Jagadeesh > > Regards, >> Andreas >> >> > >> > Regards, >> > Jagadeesh >> > >> > > >> > > Signed-off-by: Jagadeesh Krishnanjanappa >> > <jkrishnanjanappa@mvista.com <mailto:jkrishnanjanappa@mvista.com>> >> > > --- >> > > meta/recipes-connectivity/connman/connman/connman | 35 >> > +++++++++++++---------- >> > > 1 file changed, 20 insertions(+), 15 deletions(-) >> > > >> > > diff --git a/meta/recipes-connectivity/connman/connman/connman >> > b/meta/recipes-connectivity/connman/connman/connman >> > > index c64fa0d..aae2ca6 100644 >> > > --- a/meta/recipes-connectivity/connman/connman/connman >> > > +++ b/meta/recipes-connectivity/connman/connman/connman >> > > @@ -29,23 +29,28 @@ done >> > > do_start() { >> > > EXTRA_PARAM="" >> > > if test $nfsroot -eq 1 ; then >> > > - NET_DEVS=`cat /proc/net/dev | sed -ne 's/^\([a-zA-Z0-9 >> > ]*\):.*$/\1/p'` >> > > - NET_ADDR=`cat /proc/cmdline | sed -ne 's/^.*ip=\([^ >> > :]*\).*$/\1/p'` >> > > + ethn_from_dmesg=`dmesg | grep "device="| sed >> > "s|\(.*\)device=\(.*\), hwaddr=\(.*\)|\2|g"` >> > > + if [ ! -z "$ethn_from_dmesg" ]; then >> > > + EXTRA_PARAM="-I $ethn_from_dmesg" >> > > + else >> > > + NET_DEVS=`cat /proc/net/dev | sed -ne >> > 's/^\([a-zA-Z0-9 ]*\):.*$/\1/p'` >> > > + NET_ADDR=`cat /proc/cmdline | sed -ne 's/^.*ip=\([^ >> > :]*\).*$/\1/p'` >> > > >> > > - if [ ! -z "$NET_ADDR" ]; then >> > > - if [ "$NET_ADDR" = dhcp ]; then >> > > - ethn=`ifconfig | grep "^eth" | sed -e >> > "s/\(eth[0-9]\)\(.*\)/\1/"` >> > > - if [ ! -z "$ethn" ]; then >> > > - EXTRA_PARAM="-I $ethn" >> > > - fi >> > > - else >> > > - for i in $NET_DEVS; do >> > > - ADDR=`ifconfig $i | sed 's/addr://g' | sed >> > -ne 's/^.*inet \([0-9.]*\) .*$/\1/p'` >> > > - if [ "$NET_ADDR" = "$ADDR" ]; then >> > > - EXTRA_PARAM="-I $i" >> > > - break >> > > + if [ ! -z "$NET_ADDR" ]; then >> > > + if [ "$NET_ADDR" = dhcp ]; then >> > > + ethn=`ifconfig | grep "^eth" | sed -e >> > "s/\(eth[0-9]\)\(.*\)/\1/"` >> > > + if [ ! -z "$ethn" ]; then >> > > + EXTRA_PARAM="-I $ethn" >> > > fi >> > > - done >> > > + else >> > > + for i in $NET_DEVS; do >> > > + ADDR=`ifconfig $i | sed 's/addr://g' | >> > sed -ne 's/^.*inet \([0-9.]*\) .*$/\1/p'` >> > > + if [ "$NET_ADDR" = "$ADDR" ]; then >> > > + EXTRA_PARAM="-I $i" >> > > + break >> > > + fi >> > > + done >> > > + fi >> > > fi >> > > fi >> > > fi >> > > >> > >> > >> > -- >> > _______________________________________________ >> > Openembedded-core mailing list >> > Openembedded-core@lists.openembedded.org >> > <mailto:Openembedded-core@lists.openembedded.org> >> > http://lists.openembedded.org/mailman/listinfo/openembedded-core >> > <http://lists.openembedded.org/mailman/listinfo/openembedded-core> >> > >> > >> >> > [-- Attachment #2: Type: text/html, Size: 10447 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2016-10-19 18:52 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-10-10 17:35 [PATCH] connman: get the correct network interface name from dmesg during NFS booting Jagadeesh Krishnanjanappa 2016-10-11 7:41 ` Andreas Oberritter 2016-10-11 12:11 ` Jagadeesh Krishnanjanappa 2016-10-11 15:36 ` Christopher Larson 2016-10-11 19:03 ` Jagadeesh Krishnanjanappa 2016-10-11 19:34 ` Christopher Larson 2016-10-12 7:30 ` Andreas Oberritter 2016-10-12 17:49 ` Christopher Larson 2016-10-12 19:08 ` Andreas Oberritter 2016-10-12 7:00 ` Andreas Oberritter 2016-10-13 2:01 ` Jagadeesh Krishnanjanappa 2016-10-19 18:52 ` Jagadeesh Krishnanjanappa
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.