* [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 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-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-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.