[13/14] connman: do nothing in qemu, do not touch eth0

Message ID 20220109222732.2252416-13-alex@linutronix.de
State New
Headers show
Series [01/14] python3: drop unneeded multiprocessing module patch | expand

Commit Message

Alexander Kanavin Jan. 9, 2022, 10:27 p.m. UTC
qemu kernel itself is nowdays perfectly capable of setting up
what was passed in via ip=:

[    1.676847] IP-Config: Complete:
[    1.677768]      device=eth0, hwaddr=52:54:00:12:34:02, ipaddr=192.168.7.2, mask=255.255.255.0, gw=192.168.7.1
[    1.679933]      host=192.168.7.2, domain=, nis-domain=(none)
[    1.681201]      bootserver=255.255.255.255, rootserver=255.255.255.255, rootpath=
[    1.681203]      nameserver0=8.8.8.8

connman-conf only does the same thing again by (badly and incompletely)
parsing those parameters with sed.

Signed-off-by: Alexander Kanavin <alex@linutronix.de>
---
 meta/conf/distro/include/maintainers.inc      |  1 -
 meta/conf/layer.conf                          |  1 -
 .../connman/connman-conf.bb                   | 36 -------------------
 .../qemuall/wired-connection.service          | 10 ------
 .../connman/connman-conf/qemuall/wired-setup  | 16 ---------
 .../connman/connman-conf/qemuall/wired.config |  9 -----
 meta/recipes-connectivity/connman/connman.inc |  6 ++++
 .../connman/connman/connman                   |  5 ++-
 .../connman/connman_1.40.bb                   |  1 -
 9 files changed, 8 insertions(+), 77 deletions(-)
 delete mode 100644 meta/recipes-connectivity/connman/connman-conf.bb
 delete mode 100644 meta/recipes-connectivity/connman/connman-conf/qemuall/wired-connection.service
 delete mode 100644 meta/recipes-connectivity/connman/connman-conf/qemuall/wired-setup
 delete mode 100644 meta/recipes-connectivity/connman/connman-conf/qemuall/wired.config

Comments

Richard Purdie Jan. 11, 2022, 10:54 a.m. UTC | #1
On Sun, 2022-01-09 at 23:27 +0100, Alexander Kanavin wrote:
> qemu kernel itself is nowdays perfectly capable of setting up
> what was passed in via ip=:
> 
> [    1.676847] IP-Config: Complete:
> [    1.677768]      device=eth0, hwaddr=52:54:00:12:34:02, ipaddr=192.168.7.2, mask=255.255.255.0, gw=192.168.7.1
> [    1.679933]      host=192.168.7.2, domain=, nis-domain=(none)
> [    1.681201]      bootserver=255.255.255.255, rootserver=255.255.255.255, rootpath=
> [    1.681203]      nameserver0=8.8.8.8
> 
> connman-conf only does the same thing again by (badly and incompletely)
> parsing those parameters with sed.
> 
> Signed-off-by: Alexander Kanavin <alex@linutronix.de>
> 
[...]

> diff --git a/meta/recipes-connectivity/connman/connman.inc b/meta/recipes-connectivity/connman/connman.inc
> index 748eefa748..07049fbf21 100644
> --- a/meta/recipes-connectivity/connman/connman.inc
> +++ b/meta/recipes-connectivity/connman/connman.inc
> @@ -101,6 +101,12 @@ do_install:append() {
>      fi
>  }
>  
> +# Kernel IP-Config is perfectly capable of setting up networking passed in via ip=
> +do_install:append:qemuall() {
> +    mkdir -p ${D}${sysconfdir}/default
> +    echo "export EXTRA_PARAM=\"-I eth0\"" > ${D}${sysconfdir}/default/connman
> +}
> +
>  # These used to be plugins, but now they are core
>  RPROVIDES:${PN} = "\
>  	connman-plugin-loopback \

This means connman becomes machine specific, you can't do that. This would need
to be done in the connman-conf recipe.

Cheers,

Richard
Alexander Kanavin Jan. 11, 2022, 11:02 a.m. UTC | #2
On Tue, 11 Jan 2022 at 11:55, Richard Purdie <
richard.purdie@linuxfoundation.org> wrote:

> > +# Kernel IP-Config is perfectly capable of setting up networking passed
> in via ip=
> > +do_install:append:qemuall() {
> > +    mkdir -p ${D}${sysconfdir}/default
> > +    echo "export EXTRA_PARAM=\"-I eth0\"" >
> ${D}${sysconfdir}/default/connman
> > +}
>
> This means connman becomes machine specific, you can't do that. This would
> need
> to be done in the connman-conf recipe.
>

I'm not sure I follow - the above instructs connman to ignore eth0 inside
qemu (and only inside qemu), why is that problematic?

Alex
Richard Purdie Jan. 11, 2022, 11:08 a.m. UTC | #3
On Tue, 2022-01-11 at 12:02 +0100, Alexander Kanavin wrote:
> On Tue, 11 Jan 2022 at 11:55, Richard Purdie
> <richard.purdie@linuxfoundation.org> wrote:
> > > +# Kernel IP-Config is perfectly capable of setting up networking passed
> > > in
> > via ip=
> > > +do_install:append:qemuall() {
> > > +    mkdir -p ${D}${sysconfdir}/default
> > > +    echo "export EXTRA_PARAM=\"-I eth0\"" >
> > ${D}${sysconfdir}/default/connman
> > > +}
> > 
> > This means connman becomes machine specific, you can't do that. This would
> > need
> > to be done in the connman-conf recipe.
> 
> I'm not sure I follow - the above instructs connman to ignore eth0 inside qemu
> (and only inside qemu), why is that problematic?

The data above is inserted into /etc/default/connman in the connman package
itself. That package is installed into deploy/XXX/<tunearch>/connman.YYY. That
is not the machine specific package directory.

Or in different words, if I build connmand for a qemu machine, then build
connman for a machine with the same tune that isn't qemu, command rebuilds but
installs the package to the same location. This is not allowed.

Machine specific data (in this case qemu specific) shouldn't be in general
packages. This goes against yocto-check-layer and YP-Compatibility.

Cheers,

Richard
Alexander Kanavin Jan. 11, 2022, 11:10 a.m. UTC | #4
On Tue, 11 Jan 2022 at 12:08, Richard Purdie <
richard.purdie@linuxfoundation.org> wrote:

> The data above is inserted into /etc/default/connman in the connman package
> itself. That package is installed into deploy/XXX/<tunearch>/connman.YYY.
> That
> is not the machine specific package directory.
>
> Or in different words, if I build connmand for a qemu machine, then build
> connman for a machine with the same tune that isn't qemu, command rebuilds
> but
> installs the package to the same location. This is not allowed.
>
> Machine specific data (in this case qemu specific) shouldn't be in general
> packages. This goes against yocto-check-layer and YP-Compatibility.
>

Right, I can fix this, but is there a testing gap then? This prompted no
warnings on either local builds or in a-full.

Alex
Richard Purdie Jan. 11, 2022, 11:15 a.m. UTC | #5
On Tue, 2022-01-11 at 12:10 +0100, Alexander Kanavin wrote:
> On Tue, 11 Jan 2022 at 12:08, Richard Purdie
> <richard.purdie@linuxfoundation.org> wrote:
> > The data above is inserted into /etc/default/connman in the connman package
> > itself. That package is installed into deploy/XXX/<tunearch>/connman.YYY.
> > That
> > is not the machine specific package directory.
> > 
> > Or in different words, if I build connmand for a qemu machine, then build
> > connman for a machine with the same tune that isn't qemu, command rebuilds
> > but
> > installs the package to the same location. This is not allowed.
> > 
> > Machine specific data (in this case qemu specific) shouldn't be in general
> > packages. This goes against yocto-check-layer and YP-Compatibility.
> > 
> 
> 
> Right, I can fix this, but is there a testing gap then? This prompted no
> warnings on either local builds or in a-full.

The autobuilder is not definitive as it does not have 100% test coverage much as
I wish it were.

Yes, there is clearly a gap in testing here :(

We don't really have a machine with an identical tune to a qemu target that
isn't qemu which we run sstate sigs tests against.

Cheers,

Richard

Patch

diff --git a/meta/conf/distro/include/maintainers.inc b/meta/conf/distro/include/maintainers.inc
index ab7915482a..ace9894c46 100644
--- a/meta/conf/distro/include/maintainers.inc
+++ b/meta/conf/distro/include/maintainers.inc
@@ -97,7 +97,6 @@  RECIPE_MAINTAINER:pn-chrpath = "Yi Zhao <yi.zhao@windriver.com>"
 RECIPE_MAINTAINER:pn-cmake = "Pascal Bach <pascal.bach@siemens.com>"
 RECIPE_MAINTAINER:pn-cmake-native = "Pascal Bach <pascal.bach@siemens.com>"
 RECIPE_MAINTAINER:pn-connman = "Changhyeok Bae <changhyeok.bae@gmail.com>"
-RECIPE_MAINTAINER:pn-connman-conf = "Ross Burton <ross.burton@arm.com>"
 RECIPE_MAINTAINER:pn-connman-gnome = "Ross Burton <ross.burton@arm.com>"
 RECIPE_MAINTAINER:pn-consolekit = "Chen Qi <Qi.Chen@windriver.com>"
 RECIPE_MAINTAINER:pn-core-image-base = "Richard Purdie <richard.purdie@linuxfoundation.org>"
diff --git a/meta/conf/layer.conf b/meta/conf/layer.conf
index b3cc8a249e..d51ec1dc44 100644
--- a/meta/conf/layer.conf
+++ b/meta/conf/layer.conf
@@ -27,7 +27,6 @@  SIGGEN_EXCLUDERECIPES_ABISAFE += " \
   opkg-arch-config \
   netbase \
   init-ifupdown \
-  connman-conf \
   formfactor \
   xserver-xf86-config \
   pointercal-xinput \
diff --git a/meta/recipes-connectivity/connman/connman-conf.bb b/meta/recipes-connectivity/connman/connman-conf.bb
deleted file mode 100644
index 006f976997..0000000000
--- a/meta/recipes-connectivity/connman/connman-conf.bb
+++ /dev/null
@@ -1,36 +0,0 @@ 
-SUMMARY = "Connman config to setup wired interface on qemu machines"
-DESCRIPTION = "This is the ConnMan configuration to set up a Wired \
-network interface for a qemu machine."
-LICENSE = "GPLv2"
-LIC_FILES_CHKSUM = "file://${COREBASE}/meta/files/common-licenses/GPL-2.0-only;md5=801f80980d171dd6425610833a22dbe6"
-
-inherit systemd
-
-SRC_URI:append:qemuall = " file://wired.config \
-                           file://wired-setup \
-                           file://wired-connection.service \
-"
-PR = "r2"
-
-S = "${WORKDIR}"
-
-PACKAGE_ARCH = "${MACHINE_ARCH}"
-
-FILES:${PN} = "${localstatedir}/* ${datadir}/*"
-
-do_install() {
-    #Configure Wired network interface in case of qemu* machines
-    if test -e ${WORKDIR}/wired.config &&
-       test -e ${WORKDIR}/wired-setup &&
-       test -e ${WORKDIR}/wired-connection.service; then
-        install -d ${D}${localstatedir}/lib/connman
-        install -m 0644 ${WORKDIR}/wired.config ${D}${localstatedir}/lib/connman
-        install -d ${D}${datadir}/connman
-        install -m 0755 ${WORKDIR}/wired-setup ${D}${datadir}/connman
-        install -d ${D}${systemd_system_unitdir}
-        install -m 0644 ${WORKDIR}/wired-connection.service ${D}${systemd_system_unitdir}
-        sed -i -e 's|@SCRIPTDIR@|${datadir}/connman|g' ${D}${systemd_system_unitdir}/wired-connection.service
-    fi
-}
-
-SYSTEMD_SERVICE:${PN}:qemuall = "wired-connection.service"
diff --git a/meta/recipes-connectivity/connman/connman-conf/qemuall/wired-connection.service b/meta/recipes-connectivity/connman/connman-conf/qemuall/wired-connection.service
deleted file mode 100644
index 48adfc08ac..0000000000
--- a/meta/recipes-connectivity/connman/connman-conf/qemuall/wired-connection.service
+++ /dev/null
@@ -1,10 +0,0 @@ 
-[Unit]
-Description=Setup a wired interface
-Before=connman.service
-
-[Service]
-Type=oneshot
-ExecStart=@SCRIPTDIR@/wired-setup
-
-[Install]
-WantedBy=network.target
diff --git a/meta/recipes-connectivity/connman/connman-conf/qemuall/wired-setup b/meta/recipes-connectivity/connman/connman-conf/qemuall/wired-setup
deleted file mode 100644
index c46899ef32..0000000000
--- a/meta/recipes-connectivity/connman/connman-conf/qemuall/wired-setup
+++ /dev/null
@@ -1,16 +0,0 @@ 
-#!/bin/sh
-
-CONFIGF=/var/lib/connman/wired.config
-
-# Extract wired network config from /proc/cmdline
-NET_CONF=`cat /proc/cmdline |sed -ne 's/^.*ip=\([^ ]*\):\([^ ]*\):\([^ ]*\):\([^ ]*\).*$/\1\/\4\/\3/p'`
-
-# Check if eth0 is already set via kernel cmdline
-if [ "x$NET_CONF" = "x" ]; then
-	# Wired interface is not configured via kernel cmdline
-	# Remove connman config file template
-	rm -f ${CONFIGF}
-else
-	# Setup a connman config accordingly
-	sed -i -e "s|^IPv4 =.*|IPv4 = ${NET_CONF}|" ${CONFIGF}
-fi
diff --git a/meta/recipes-connectivity/connman/connman-conf/qemuall/wired.config b/meta/recipes-connectivity/connman/connman-conf/qemuall/wired.config
deleted file mode 100644
index 42998ce897..0000000000
--- a/meta/recipes-connectivity/connman/connman-conf/qemuall/wired.config
+++ /dev/null
@@ -1,9 +0,0 @@ 
-[global]
-Name = Wired
-Description = Wired network configuration
-
-[service_ethernet]
-Type = ethernet
-IPv4 =
-MAC = 52:54:00:12:34:56
-Nameservers = 8.8.8.8
diff --git a/meta/recipes-connectivity/connman/connman.inc b/meta/recipes-connectivity/connman/connman.inc
index 748eefa748..07049fbf21 100644
--- a/meta/recipes-connectivity/connman/connman.inc
+++ b/meta/recipes-connectivity/connman/connman.inc
@@ -101,6 +101,12 @@  do_install:append() {
     fi
 }
 
+# Kernel IP-Config is perfectly capable of setting up networking passed in via ip=
+do_install:append:qemuall() {
+    mkdir -p ${D}${sysconfdir}/default
+    echo "export EXTRA_PARAM=\"-I eth0\"" > ${D}${sysconfdir}/default/connman
+}
+
 # These used to be plugins, but now they are core
 RPROVIDES:${PN} = "\
 	connman-plugin-loopback \
diff --git a/meta/recipes-connectivity/connman/connman/connman b/meta/recipes-connectivity/connman/connman/connman
index c64fa0d715..310a696863 100644
--- a/meta/recipes-connectivity/connman/connman/connman
+++ b/meta/recipes-connectivity/connman/connman/connman
@@ -27,7 +27,6 @@  while read dev mtpt fstype rest; do
 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'`
@@ -36,13 +35,13 @@  do_start() {
 		if [ "$NET_ADDR" = dhcp ]; then
 		    ethn=`ifconfig | grep "^eth" | sed -e "s/\(eth[0-9]\)\(.*\)/\1/"`
 		    if [ ! -z "$ethn" ]; then
-			EXTRA_PARAM="-I $ethn"
+			EXTRA_PARAM="$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"
+			    EXTRA_PARAM="$EXTRA_PARAM -I $i"
 			    break
 			fi
 		    done
diff --git a/meta/recipes-connectivity/connman/connman_1.40.bb b/meta/recipes-connectivity/connman/connman_1.40.bb
index edb23a1267..84cc3959d1 100644
--- a/meta/recipes-connectivity/connman/connman_1.40.bb
+++ b/meta/recipes-connectivity/connman/connman_1.40.bb
@@ -11,5 +11,4 @@  SRC_URI:append:libc-musl = " file://0002-resolve-musl-does-not-implement-res_nin
 
 SRC_URI[sha256sum] = "1a57ae7ce234aa3a1744aac3be5c2121d98dce999440ef8ab9cc4edfd5edcb12"
 
-RRECOMMENDS:${PN} = "connman-conf"
 RCONFLICTS:${PN} = "networkmanager"