diff mbox series

openssh: Add SSHD_SERVICE_TYPE variable

Message ID 20231204085408.2125531-1-mingli.yu@windriver.com
State New
Headers show
Series openssh: Add SSHD_SERVICE_TYPE variable | expand

Commit Message

Yu, Mingli Dec. 4, 2023, 8:54 a.m. UTC
From: Mingli Yu <mingli.yu@windriver.com>

There are two types of sshd server now, one is based on socket
activation(sshd.socket) and another is service activation(sshd.service).
And the default sshd service type is based on socket by default as below.
============================================
 # systemctl status sshd.socket
* sshd.socket
     Loaded: loaded (/lib/systemd/system/sshd.socket; enabled; preset: enabled)
     Active: active (listening) since Mon 2023-12-04 08:34:33 UTC; 22s ago
     Listen: [::]:22 (Stream)
   Accepted: 0; Connected: 0;
    Process: 228 ExecStartPre=/bin/mkdir -p /var/run/sshd (code=exited, status=0/SUCCESS)
      Tasks: 0 (limit: 263)
     Memory: 84.0K
     CGroup: /system.slice/sshd.socket

Dec 04 08:34:33 qemux86-64 systemd[1]: Starting sshd.socket...
Dec 04 08:34:33 qemux86-64 systemd[1]: Listening on sshd.socket.
============================================

And use can switch to service activation if they want as below after
the device boot up.
============================================
 # systemctl disable sshd.socket
Removed "/etc/systemd/system/sockets.target.wants/sshd.socket".
 # systemctl stop sshd.socket
 # systemctl start sshd.service
 # systemctl status sshd.service
* sshd.service - OpenSSH server daemon
     Loaded: loaded (/lib/systemd/system/sshd.service; enabled; preset: enabled)
     Active: active (running) since Mon 2023-12-04 08:48:14 UTC; 53s ago
    Process: 390 ExecStartPre=/bin/mkdir -p /var/run/sshd (code=exited, status=0/SUCCESS)
   Main PID: 391 (sshd)
      Tasks: 1 (limit: 263)
     Memory: 2.1M
     CGroup: /system.slice/sshd.service
             `-391 "sshd: /usr/sbin/sshd -D [listener] 0 of 10-100 startups"

Dec 04 08:48:14 qemux86-64 systemd[1]: Starting OpenSSH server daemon...
Dec 04 08:48:14 qemux86-64 systemd[1]: Started OpenSSH server daemon.
Dec 04 08:48:14 qemux86-64 sshd[391]: Server listening on 0.0.0.0 port 22.
Dec 04 08:48:14 qemux86-64 sshd[391]: Server listening on :: port 22.
============================================

But it's more convenient to determine the service type at build time
if there are so many devices.

So add SSHD_SERVICE_TYPE variable to enable sshd.socket or sshd.service
at build time and we still enable sshd.socket by default now.

Signed-off-by: Mingli Yu <mingli.yu@windriver.com>
---
 .../openssh/openssh_9.5p1.bb                   | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

Comments

Richard Purdie Dec. 4, 2023, 11:07 a.m. UTC | #1
On Mon, 2023-12-04 at 16:54 +0800, Yu, Mingli wrote:
> From: Mingli Yu <mingli.yu@windriver.com>
> 
> There are two types of sshd server now, one is based on socket
> activation(sshd.socket) and another is service activation(sshd.service).
> And the default sshd service type is based on socket by default as below.

Firstly, I'm a little frustrated about how these changes are being
proposed in pieces. The original patches made no mention that this was
eventually going to be controlled by a new variable. I did wonder if
something like this was going to happen but the didn't push back. Now
we're "pinned" into a corner with the direction this code takes now
we've already merged the on target configuration change.

> But it's more convenient to determine the service type at build time
> if there are so many devices.

Why did we merge a patch which made this an on target decision then?

> So add SSHD_SERVICE_TYPE variable to enable sshd.socket or sshd.service
> at build time and we still enable sshd.socket by default now.

No, we're not adding randomly named variables which take magic options.

I cannot look at SSHD_SERVICE_TYPE and know that "1" means socket
activation and not setting it means service activation (or whatever,
the fact I can't check what I've written easily proves my point). What
happens if I set it to "true", or "socket", or "apples"?

I'd suggest thinking about a PACKAGECONFIG option such as systemd-sshd-
socket-mode and systemd-sshd-service-mode which at least uses naming
users are mode used to and says what it does.

Cheers,

Richard
Yu, Mingli Dec. 5, 2023, 2:51 a.m. UTC | #2
Hi Richard,

On 12/4/23 19:07, Richard Purdie wrote:
> CAUTION: This email comes from a non Wind River email account!
> Do not click links or open attachments unless you recognize the sender and know the content is safe.
> 
> On Mon, 2023-12-04 at 16:54 +0800, Yu, Mingli wrote:
>> From: Mingli Yu <mingli.yu@windriver.com>
>>
>> There are two types of sshd server now, one is based on socket
>> activation(sshd.socket) and another is service activation(sshd.service).
>> And the default sshd service type is based on socket by default as below.
> 
> Firstly, I'm a little frustrated about how these changes are being
> proposed in pieces. The original patches made no mention that this was
> eventually going to be controlled by a new variable. I did wonder if
> something like this was going to happen but the didn't push back. Now
> we're "pinned" into a corner with the direction this code takes now
> we've already merged the on target configuration change.
> 
>> But it's more convenient to determine the service type at build time
>> if there are so many devices.
> 
> Why did we merge a patch which made this an on target decision then?

Maybe usage requirement change? sorry for noise´╝ü

> 
>> So add SSHD_SERVICE_TYPE variable to enable sshd.socket or sshd.service
>> at build time and we still enable sshd.socket by default now.
> 
> No, we're not adding randomly named variables which take magic options.
> 
> I cannot look at SSHD_SERVICE_TYPE and know that "1" means socket
> activation and not setting it means service activation (or whatever,
> the fact I can't check what I've written easily proves my point). What
> happens if I set it to "true", or "socket", or "apples"?
> 
> I'd suggest thinking about a PACKAGECONFIG option such as systemd-sshd-
> socket-mode and systemd-sshd-service-mode which at least uses naming
> users are mode used to and says what it does.

Will sent v2 to use the common PACKAGECONFIG option to control the sshd 
type.

Thanks,

> 
> Cheers,
> 
> Richard
diff mbox series

Patch

diff --git a/meta/recipes-connectivity/openssh/openssh_9.5p1.bb b/meta/recipes-connectivity/openssh/openssh_9.5p1.bb
index bbb8fb091a..6a603cd12d 100644
--- a/meta/recipes-connectivity/openssh/openssh_9.5p1.bb
+++ b/meta/recipes-connectivity/openssh/openssh_9.5p1.bb
@@ -50,7 +50,7 @@  INITSCRIPT_NAME:${PN}-sshd = "sshd"
 INITSCRIPT_PARAMS:${PN}-sshd = "defaults 9"
 
 SYSTEMD_PACKAGES = "${PN}-sshd"
-SYSTEMD_SERVICE:${PN}-sshd = "sshd.socket sshd.service"
+SYSTEMD_SERVICE:${PN}-sshd = "${@bb.utils.contains('SSHD_SERVICE_TYPE', '1', 'sshd.service', 'sshd.socket', d)}"
 
 inherit autotools-brokensep ptest pkgconfig
 DEPENDS += "${@bb.utils.contains('DISTRO_FEATURES', 'systemd', 'systemd', '', d)}"
@@ -125,15 +125,23 @@  do_install:append () {
 	echo "HostKey /var/run/ssh/ssh_host_ed25519_key" >> ${D}${sysconfdir}/ssh/sshd_config_readonly
 
 	install -d ${D}${systemd_system_unitdir}
-	install -c -m 0644 ${WORKDIR}/sshd.socket ${D}${systemd_system_unitdir}
-	install -c -m 0644 ${WORKDIR}/sshd.service ${D}${systemd_system_unitdir}
-	install -c -m 0644 ${WORKDIR}/sshd@.service ${D}${systemd_system_unitdir}
+	if ${@bb.utils.contains('SSHD_SERVICE_TYPE','1','true','false',d)}; then
+	    install -c -m 0644 ${WORKDIR}/sshd.service ${D}${systemd_system_unitdir}
+	else
+	    install -c -m 0644 ${WORKDIR}/sshd.socket ${D}${systemd_system_unitdir}
+	    install -c -m 0644 ${WORKDIR}/sshd@.service ${D}${systemd_system_unitdir}
+	    sed -i -e 's,@BASE_BINDIR@,${base_bindir},g' \
+		    -e 's,@SBINDIR@,${sbindir},g' \
+		    -e 's,@BINDIR@,${bindir},g' \
+		    -e 's,@LIBEXECDIR@,${libexecdir}/${BPN},g' \
+            ${D}${systemd_system_unitdir}/sshd.socket
+	fi
 	install -c -m 0644 ${WORKDIR}/sshdgenkeys.service ${D}${systemd_system_unitdir}
 	sed -i -e 's,@BASE_BINDIR@,${base_bindir},g' \
 		-e 's,@SBINDIR@,${sbindir},g' \
 		-e 's,@BINDIR@,${bindir},g' \
 		-e 's,@LIBEXECDIR@,${libexecdir}/${BPN},g' \
-		${D}${systemd_system_unitdir}/sshd.socket ${D}${systemd_system_unitdir}/*.service
+		${D}${systemd_system_unitdir}/*.service
 
 	sed -i -e 's,@LIBEXECDIR@,${libexecdir}/${BPN},g' \
 		${D}${sysconfdir}/init.d/sshd