[PATCH 0/4] slirp: Update submodule to include NC-SI features

[PATCH 0/4] slirp: Update submodule to include NC-SI features

[Peter Delevoryas]

I recently submitted some NC-SI changes ([1], [2]) to libslirp, and I'd like
to include them as netdev options in QEMU now.

I'm guessing that before we can do that though, there probably needs
to be a new libslirp release, right? Right now we're on 4.7.0, with
SlirpConfig version 4. I guess I'd like to create (or wait, either
is fine) 4.8.0, and update QEMU to use SlirpConfig version 5 if
that version is available.

By the way: do I need to add #if SLIRP_CHECK_VERSION(4,7,0) around any code
I'm adding? I'm hoping that's not the case, but I see that was necessary
with the recent timer changes.

Let me know what I should do, I am fully expecting that I'll need to either
wait for the next slirp release, submit a v2, etc. Just wanted to get the
ball rolling.

Thanks,
Peter

[1] https://gitlab.freedesktop.org/slirp/libslirp/-/merge_requests/122
[2] https://gitlab.freedesktop.org/slirp/libslirp/-/merge_requests/125



Peter Delevoryas (4):
  slirp: Update submodule to include NC-SI updates
  slirp: Update SlirpConfig version to 5
  slirp: Add mfr-id to -netdev options
  slirp: Add oob-eth-addr to -netdev options

 net/slirp.c   | 18 ++++++++++++++----
 qapi/net.json |  8 +++++++-
 slirp         |  2 +-
 3 files changed, 22 insertions(+), 6 deletions(-)

-- 
2.30.2

[PATCH 1/4] slirp: Update submodule to include NC-SI updates

[Peter Delevoryas]

There hasn't been a new libslirp release yet, but I don't see any reason
in particular why we couldn't update to this commit to support the new
NC-SI functionality added in !122 and !125, as long as it doesn't break
backwards compatibility with anything.

[1] https://gitlab.freedesktop.org/slirp/libslirp/-/merge_requests/122
[2] https://gitlab.freedesktop.org/slirp/libslirp/-/merge_requests/125

Signed-off-by: Peter Delevoryas <pdel@fb.com>
---
 slirp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/slirp b/slirp
index 9d59bb775d..baf336047c 160000
--- a/slirp
+++ b/slirp
@@ -1 +1 @@
-Subproject commit 9d59bb775d6294c8b447a88512f7bb43f12a25a8
+Subproject commit baf336047c3bb869b3103a6b1b3d8b15e5f1c58d
-- 
2.30.2

[PATCH 2/4] slirp: Update SlirpConfig version to 5

[Peter Delevoryas]

I think we probably need a new Slirp release
(4.8.0) and a switch statement here instead, right?

So that we can preserve the behavior for 4.7.0?

Signed-off-by: Peter Delevoryas <pdel@fb.com>
---
 net/slirp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/slirp.c b/net/slirp.c
index 8679be6444..75e5ccafd9 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -617,7 +617,7 @@ static int net_slirp_init(NetClientState *peer, const char *model,
 
     s = DO_UPCAST(SlirpState, nc, nc);
 
-    cfg.version = SLIRP_CHECK_VERSION(4,7,0) ? 4 : 1;
+    cfg.version = SLIRP_CHECK_VERSION(4, 7, 0) ? 5 : 1;
     cfg.restricted = restricted;
     cfg.in_enabled = ipv4;
     cfg.vnetwork = net;
-- 
2.30.2

[PATCH 3/4] slirp: Add mfr-id to -netdev options

[Peter Delevoryas]

This lets you set the manufacturer's ID for a slirp netdev, which can be
queried from the guest through the Get Version ID NC-SI command. For
example, by setting the manufacturer's ID to 0x8119:

    wget https://github.com/facebook/openbmc/releases/download/openbmc-e2294ff5d31d/fby35.mtd
    qemu-system-arm -machine fby35-bmc \
        -drive file=fby35.mtd,format=raw,if=mtd -nographic \
        -netdev user,id=nic,mfr-id=0x8119,hostfwd=::2222-:22 \
        -net nic,model=ftgmac100,netdev=nic
    ...
    username: root
    password: 0penBmc
    ...
    root@bmc-oob:~# ncsi-util 0x15
    NC-SI Command Response:
    cmd: GET_VERSION_ID(0x15)
    Response: COMMAND_COMPLETED(0x0000)  Reason: NO_ERROR(0x0000)
    Payload length = 40

    20: 0xf1 0xf0 0xf0 0x00
    24: 0x00 0x00 0x00 0x00
    28: 0x00 0x00 0x00 0x00
    32: 0x00 0x00 0x00 0x00
    36: 0x00 0x00 0x00 0x00
    40: 0x00 0x00 0x00 0x00
    44: 0x00 0x00 0x00 0x00
    48: 0x00 0x00 0x00 0x00
    52: 0x00 0x00 0x81 0x19

Signed-off-by: Peter Delevoryas <pdel@fb.com>
---
 net/slirp.c   | 5 +++--
 qapi/net.json | 5 ++++-
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/net/slirp.c b/net/slirp.c
index 75e5ccafd9..231068c1e2 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -413,7 +413,7 @@ static int net_slirp_init(NetClientState *peer, const char *model,
                           const char *vnameserver, const char *vnameserver6,
                           const char *smb_export, const char *vsmbserver,
                           const char **dnssearch, const char *vdomainname,
-                          const char *tftp_server_name,
+                          const char *tftp_server_name, uint32_t mfr_id,
                           Error **errp)
 {
     /* default settings according to historic slirp */
@@ -636,6 +636,7 @@ static int net_slirp_init(NetClientState *peer, const char *model,
     cfg.vnameserver6 = ip6_dns;
     cfg.vdnssearch = dnssearch;
     cfg.vdomainname = vdomainname;
+    cfg.mfr_id = mfr_id;
     s->slirp = slirp_new(&cfg, &slirp_cb, s);
     QTAILQ_INSERT_TAIL(&slirp_stacks, s, entry);
 
@@ -1172,7 +1173,7 @@ int net_init_slirp(const Netdev *netdev, const char *name,
                          user->bootfile, user->dhcpstart,
                          user->dns, user->ipv6_dns, user->smb,
                          user->smbserver, dnssearch, user->domainname,
-                         user->tftp_server_name, errp);
+                         user->tftp_server_name, user->mfr_id, errp);
 
     while (slirp_configs) {
         config = slirp_configs;
diff --git a/qapi/net.json b/qapi/net.json
index d6f7cfd4d6..efc5cb3fb6 100644
--- a/qapi/net.json
+++ b/qapi/net.json
@@ -167,6 +167,8 @@
 #
 # @tftp-server-name: RFC2132 "TFTP server name" string (Since 3.1)
 #
+# @mfr-id: Manufacturer ID (Private Enterprise Number: IANA)
+#
 # Since: 1.2
 ##
 { 'struct': 'NetdevUserOptions',
@@ -192,7 +194,8 @@
     '*smbserver': 'str',
     '*hostfwd':   ['String'],
     '*guestfwd':  ['String'],
-    '*tftp-server-name': 'str' } }
+    '*tftp-server-name': 'str',
+    '*mfr-id': 'uint32' } }
 
 ##
 # @NetdevTapOptions:
-- 
2.30.2

[PATCH 4/4] slirp: Add oob-eth-addr to -netdev options

[Peter Delevoryas]

With this change, you can now request the out-of-band MAC address from
slirp in fby35-bmc:

    wget https://github.com/facebook/openbmc/releases/download/openbmc-e2294ff5d31d/fby35.mtd
    qemu-system-arm -machine fby35-bmc \
        -drive file=fby35.mtd,format=raw,if=mtd \
        -nographic \
        -netdev user,id=nic,mfr-id=0x8119,oob-eth-addr=de:ad:be:ef:ca:fe,hostfwd=::2222-:22 \
        -net nic,model=ftgmac100,netdev=nic

    ...
    username: root
    password: 0penBmc
    ...

    root@bmc-oob:~# ncsi-util -n eth0 -c 0 0x50 0 0 0x81 0x19 0 0 0x1b 0
    NC-SI Command Response:
    cmd: NCSI_OEM_CMD(0x50)
    Response: COMMAND_COMPLETED(0x0000)  Reason: NO_ERROR(0x0000)
    Payload length = 24

    20: 0x00 0x00 0x81 0x19
    24: 0x01 0x00 0x1b 0x00
    28: 0x00 0x00 0x00 0x00
    32: 0xde 0xad 0xbe 0xef
    36: 0xca 0xfe 0x00 0x00

    root@bmc-oob:~# ifconfig
    eth0      Link encap:Ethernet  HWaddr DE:AD:BE:EF:CA:FE
	      inet addr:10.0.2.15  Bcast:10.0.2.255  Mask:255.255.255.0
	      inet6 addr: fec0::dcad:beff:feef:cafe/64 Scope:Site
	      inet6 addr: fe80::dcad:beff:feef:cafe/64 Scope:Link
	      UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
	      RX packets:253 errors:0 dropped:0 overruns:0 frame:0
	      TX packets:271 errors:0 dropped:0 overruns:0 carrier:0
	      collisions:0 txqueuelen:1000
	      RX bytes:24638 (24.0 KiB)  TX bytes:18876 (18.4 KiB)
	      Interrupt:32

    lo        Link encap:Local Loopback
	      inet addr:127.0.0.1  Mask:255.0.0.0
	      inet6 addr: ::1/128 Scope:Host
	      UP LOOPBACK RUNNING  MTU:65536  Metric:1
	      RX packets:2 errors:0 dropped:0 overruns:0 frame:0
	      TX packets:2 errors:0 dropped:0 overruns:0 carrier:0
	      collisions:0 txqueuelen:1000
	      RX bytes:120 (120.0 B)  TX bytes:120 (120.0 B)

Signed-off-by: Peter Delevoryas <pdel@fb.com>
---
 net/slirp.c   | 13 +++++++++++--
 qapi/net.json |  5 ++++-
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/net/slirp.c b/net/slirp.c
index 231068c1e2..858d3da859 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -414,7 +414,7 @@ static int net_slirp_init(NetClientState *peer, const char *model,
                           const char *smb_export, const char *vsmbserver,
                           const char **dnssearch, const char *vdomainname,
                           const char *tftp_server_name, uint32_t mfr_id,
-                          Error **errp)
+                          uint8_t oob_eth_addr[ETH_ALEN], Error **errp)
 {
     /* default settings according to historic slirp */
     struct in_addr net  = { .s_addr = htonl(0x0a000200) }; /* 10.0.2.0 */
@@ -637,6 +637,7 @@ static int net_slirp_init(NetClientState *peer, const char *model,
     cfg.vdnssearch = dnssearch;
     cfg.vdomainname = vdomainname;
     cfg.mfr_id = mfr_id;
+    memcpy(cfg.oob_eth_addr, oob_eth_addr, ETH_ALEN);
     s->slirp = slirp_new(&cfg, &slirp_cb, s);
     QTAILQ_INSERT_TAIL(&slirp_stacks, s, entry);
 
@@ -1142,6 +1143,7 @@ int net_init_slirp(const Netdev *netdev, const char *name,
     const NetdevUserOptions *user;
     const char **dnssearch;
     bool ipv4 = true, ipv6 = true;
+    MACAddr oob_eth_addr = {};
 
     assert(netdev->type == NET_CLIENT_DRIVER_USER);
     user = &netdev->u.user;
@@ -1166,6 +1168,12 @@ int net_init_slirp(const Netdev *netdev, const char *name,
     net_init_slirp_configs(user->hostfwd, SLIRP_CFG_HOSTFWD);
     net_init_slirp_configs(user->guestfwd, 0);
 
+    if (user->has_oob_eth_addr &&
+        net_parse_macaddr(oob_eth_addr.a, user->oob_eth_addr) < 0) {
+        error_setg(errp, "invalid syntax for OOB ethernet address");
+        return -1;
+    }
+
     ret = net_slirp_init(peer, "user", name, user->q_restrict,
                          ipv4, vnet, user->host,
                          ipv6, user->ipv6_prefix, user->ipv6_prefixlen,
@@ -1173,7 +1181,8 @@ int net_init_slirp(const Netdev *netdev, const char *name,
                          user->bootfile, user->dhcpstart,
                          user->dns, user->ipv6_dns, user->smb,
                          user->smbserver, dnssearch, user->domainname,
-                         user->tftp_server_name, user->mfr_id, errp);
+                         user->tftp_server_name, user->mfr_id, oob_eth_addr.a,
+                         errp);
 
     while (slirp_configs) {
         config = slirp_configs;
diff --git a/qapi/net.json b/qapi/net.json
index efc5cb3fb6..7b2c3c205c 100644
--- a/qapi/net.json
+++ b/qapi/net.json
@@ -169,6 +169,8 @@
 #
 # @mfr-id: Manufacturer ID (Private Enterprise Number: IANA)
 #
+# @oob-eth-addr: Out-of-band ethernet address for BMC
+#
 # Since: 1.2
 ##
 { 'struct': 'NetdevUserOptions',
@@ -195,7 +197,8 @@
     '*hostfwd':   ['String'],
     '*guestfwd':  ['String'],
     '*tftp-server-name': 'str',
-    '*mfr-id': 'uint32' } }
+    '*mfr-id': 'uint32',
+    '*oob-eth-addr': 'str' } }
 
 ##
 # @NetdevTapOptions:
-- 
2.30.2

Re: [PATCH 2/4] slirp: Update SlirpConfig version to 5

[Samuel Thibault]

Hello,

Peter Delevoryas, le mer. 15 juin 2022 18:05:24 -0700, a ecrit:
> I think we probably need a new Slirp release
> (4.8.0) and a switch statement here instead, right?
> 
> So that we can preserve the behavior for 4.7.0?

Yes, that's the idea.

Samuel

Re: [PATCH 3/4] slirp: Add mfr-id to -netdev options

[Samuel Thibault]

Peter Delevoryas, le mer. 15 juin 2022 18:05:25 -0700, a ecrit:
> This lets you set the manufacturer's ID for a slirp netdev, which can be
> queried from the guest through the Get Version ID NC-SI command. For
> example, by setting the manufacturer's ID to 0x8119:
> 
>     wget https://github.com/facebook/openbmc/releases/download/openbmc-e2294ff5d31d/fby35.mtd
>     qemu-system-arm -machine fby35-bmc \
>         -drive file=fby35.mtd,format=raw,if=mtd -nographic \
>         -netdev user,id=nic,mfr-id=0x8119,hostfwd=::2222-:22 \
>         -net nic,model=ftgmac100,netdev=nic
>     ...
>     username: root
>     password: 0penBmc
>     ...
>     root@bmc-oob:~# ncsi-util 0x15
>     NC-SI Command Response:
>     cmd: GET_VERSION_ID(0x15)
>     Response: COMMAND_COMPLETED(0x0000)  Reason: NO_ERROR(0x0000)
>     Payload length = 40
> 
>     20: 0xf1 0xf0 0xf0 0x00
>     24: 0x00 0x00 0x00 0x00
>     28: 0x00 0x00 0x00 0x00
>     32: 0x00 0x00 0x00 0x00
>     36: 0x00 0x00 0x00 0x00
>     40: 0x00 0x00 0x00 0x00
>     44: 0x00 0x00 0x00 0x00
>     48: 0x00 0x00 0x00 0x00
>     52: 0x00 0x00 0x81 0x19
> 
> Signed-off-by: Peter Delevoryas <pdel@fb.com>
> ---
>  net/slirp.c   | 5 +++--
>  qapi/net.json | 5 ++++-
>  2 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/net/slirp.c b/net/slirp.c
> index 75e5ccafd9..231068c1e2 100644
> --- a/net/slirp.c
> +++ b/net/slirp.c
> @@ -413,7 +413,7 @@ static int net_slirp_init(NetClientState *peer, const char *model,
>                            const char *vnameserver, const char *vnameserver6,
>                            const char *smb_export, const char *vsmbserver,
>                            const char **dnssearch, const char *vdomainname,
> -                          const char *tftp_server_name,
> +                          const char *tftp_server_name, uint32_t mfr_id,
>                            Error **errp)
>  {
>      /* default settings according to historic slirp */
> @@ -636,6 +636,7 @@ static int net_slirp_init(NetClientState *peer, const char *model,
>      cfg.vnameserver6 = ip6_dns;
>      cfg.vdnssearch = dnssearch;
>      cfg.vdomainname = vdomainname;
> +    cfg.mfr_id = mfr_id;

You will need a #if to only include it with slirp 4.8.0 indeed.
Otherwise the mfr_id field won't exist.

In the #else part, it would probably useful to give the user at least a
warning that tells her to upgrade slirp to 4.8.0 to get the mfr_id
functionality working.

>      s->slirp = slirp_new(&cfg, &slirp_cb, s);
>      QTAILQ_INSERT_TAIL(&slirp_stacks, s, entry);
>  
> @@ -1172,7 +1173,7 @@ int net_init_slirp(const Netdev *netdev, const char *name,
>                           user->bootfile, user->dhcpstart,
>                           user->dns, user->ipv6_dns, user->smb,
>                           user->smbserver, dnssearch, user->domainname,
> -                         user->tftp_server_name, errp);
> +                         user->tftp_server_name, user->mfr_id, errp);
>  
>      while (slirp_configs) {
>          config = slirp_configs;
> diff --git a/qapi/net.json b/qapi/net.json
> index d6f7cfd4d6..efc5cb3fb6 100644
> --- a/qapi/net.json
> +++ b/qapi/net.json
> @@ -167,6 +167,8 @@
>  #
>  # @tftp-server-name: RFC2132 "TFTP server name" string (Since 3.1)
>  #
> +# @mfr-id: Manufacturer ID (Private Enterprise Number: IANA)
> +#
>  # Since: 1.2
>  ##
>  { 'struct': 'NetdevUserOptions',
> @@ -192,7 +194,8 @@
>      '*smbserver': 'str',
>      '*hostfwd':   ['String'],
>      '*guestfwd':  ['String'],
> -    '*tftp-server-name': 'str' } }
> +    '*tftp-server-name': 'str',
> +    '*mfr-id': 'uint32' } }
>  
>  ##
>  # @NetdevTapOptions:
> -- 
> 2.30.2
> 

Re: [PATCH 4/4] slirp: Add oob-eth-addr to -netdev options

[Samuel Thibault]

Peter Delevoryas, le mer. 15 juin 2022 18:05:26 -0700, a ecrit:
> With this change, you can now request the out-of-band MAC address from
> slirp in fby35-bmc:
> 
>     wget https://github.com/facebook/openbmc/releases/download/openbmc-e2294ff5d31d/fby35.mtd
>     qemu-system-arm -machine fby35-bmc \
>         -drive file=fby35.mtd,format=raw,if=mtd \
>         -nographic \
>         -netdev user,id=nic,mfr-id=0x8119,oob-eth-addr=de:ad:be:ef:ca:fe,hostfwd=::2222-:22 \
>         -net nic,model=ftgmac100,netdev=nic
> 
>     ...
>     username: root
>     password: 0penBmc
>     ...
> 
>     root@bmc-oob:~# ncsi-util -n eth0 -c 0 0x50 0 0 0x81 0x19 0 0 0x1b 0
>     NC-SI Command Response:
>     cmd: NCSI_OEM_CMD(0x50)
>     Response: COMMAND_COMPLETED(0x0000)  Reason: NO_ERROR(0x0000)
>     Payload length = 24
> 
>     20: 0x00 0x00 0x81 0x19
>     24: 0x01 0x00 0x1b 0x00
>     28: 0x00 0x00 0x00 0x00
>     32: 0xde 0xad 0xbe 0xef
>     36: 0xca 0xfe 0x00 0x00
> 
>     root@bmc-oob:~# ifconfig
>     eth0      Link encap:Ethernet  HWaddr DE:AD:BE:EF:CA:FE
> 	      inet addr:10.0.2.15  Bcast:10.0.2.255  Mask:255.255.255.0
> 	      inet6 addr: fec0::dcad:beff:feef:cafe/64 Scope:Site
> 	      inet6 addr: fe80::dcad:beff:feef:cafe/64 Scope:Link
> 	      UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
> 	      RX packets:253 errors:0 dropped:0 overruns:0 frame:0
> 	      TX packets:271 errors:0 dropped:0 overruns:0 carrier:0
> 	      collisions:0 txqueuelen:1000
> 	      RX bytes:24638 (24.0 KiB)  TX bytes:18876 (18.4 KiB)
> 	      Interrupt:32
> 
>     lo        Link encap:Local Loopback
> 	      inet addr:127.0.0.1  Mask:255.0.0.0
> 	      inet6 addr: ::1/128 Scope:Host
> 	      UP LOOPBACK RUNNING  MTU:65536  Metric:1
> 	      RX packets:2 errors:0 dropped:0 overruns:0 frame:0
> 	      TX packets:2 errors:0 dropped:0 overruns:0 carrier:0
> 	      collisions:0 txqueuelen:1000
> 	      RX bytes:120 (120.0 B)  TX bytes:120 (120.0 B)
> 
> Signed-off-by: Peter Delevoryas <pdel@fb.com>
> ---
>  net/slirp.c   | 13 +++++++++++--
>  qapi/net.json |  5 ++++-
>  2 files changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/net/slirp.c b/net/slirp.c
> index 231068c1e2..858d3da859 100644
> --- a/net/slirp.c
> +++ b/net/slirp.c
> @@ -414,7 +414,7 @@ static int net_slirp_init(NetClientState *peer, const char *model,
>                            const char *smb_export, const char *vsmbserver,
>                            const char **dnssearch, const char *vdomainname,
>                            const char *tftp_server_name, uint32_t mfr_id,
> -                          Error **errp)
> +                          uint8_t oob_eth_addr[ETH_ALEN], Error **errp)
>  {
>      /* default settings according to historic slirp */
>      struct in_addr net  = { .s_addr = htonl(0x0a000200) }; /* 10.0.2.0 */
> @@ -637,6 +637,7 @@ static int net_slirp_init(NetClientState *peer, const char *model,
>      cfg.vdnssearch = dnssearch;
>      cfg.vdomainname = vdomainname;
>      cfg.mfr_id = mfr_id;
> +    memcpy(cfg.oob_eth_addr, oob_eth_addr, ETH_ALEN);

And similarly here.

>      s->slirp = slirp_new(&cfg, &slirp_cb, s);
>      QTAILQ_INSERT_TAIL(&slirp_stacks, s, entry);
>  
> @@ -1142,6 +1143,7 @@ int net_init_slirp(const Netdev *netdev, const char *name,
>      const NetdevUserOptions *user;
>      const char **dnssearch;
>      bool ipv4 = true, ipv6 = true;
> +    MACAddr oob_eth_addr = {};
>  
>      assert(netdev->type == NET_CLIENT_DRIVER_USER);
>      user = &netdev->u.user;
> @@ -1166,6 +1168,12 @@ int net_init_slirp(const Netdev *netdev, const char *name,
>      net_init_slirp_configs(user->hostfwd, SLIRP_CFG_HOSTFWD);
>      net_init_slirp_configs(user->guestfwd, 0);
>  
> +    if (user->has_oob_eth_addr &&
> +        net_parse_macaddr(oob_eth_addr.a, user->oob_eth_addr) < 0) {
> +        error_setg(errp, "invalid syntax for OOB ethernet address");
> +        return -1;
> +    }
> +
>      ret = net_slirp_init(peer, "user", name, user->q_restrict,
>                           ipv4, vnet, user->host,
>                           ipv6, user->ipv6_prefix, user->ipv6_prefixlen,
> @@ -1173,7 +1181,8 @@ int net_init_slirp(const Netdev *netdev, const char *name,
>                           user->bootfile, user->dhcpstart,
>                           user->dns, user->ipv6_dns, user->smb,
>                           user->smbserver, dnssearch, user->domainname,
> -                         user->tftp_server_name, user->mfr_id, errp);
> +                         user->tftp_server_name, user->mfr_id, oob_eth_addr.a,
> +                         errp);
>  
>      while (slirp_configs) {
>          config = slirp_configs;
> diff --git a/qapi/net.json b/qapi/net.json
> index efc5cb3fb6..7b2c3c205c 100644
> --- a/qapi/net.json
> +++ b/qapi/net.json
> @@ -169,6 +169,8 @@
>  #
>  # @mfr-id: Manufacturer ID (Private Enterprise Number: IANA)
>  #
> +# @oob-eth-addr: Out-of-band ethernet address for BMC
> +#
>  # Since: 1.2
>  ##
>  { 'struct': 'NetdevUserOptions',
> @@ -195,7 +197,8 @@
>      '*hostfwd':   ['String'],
>      '*guestfwd':  ['String'],
>      '*tftp-server-name': 'str',
> -    '*mfr-id': 'uint32' } }
> +    '*mfr-id': 'uint32',
> +    '*oob-eth-addr': 'str' } }
>  
>  ##
>  # @NetdevTapOptions:
> -- 
> 2.30.2
> 

-- 
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.

Re: [PATCH 3/4] slirp: Add mfr-id to -netdev options

[Markus Armbruster]

Peter Delevoryas <pdel@fb.com> writes:

> This lets you set the manufacturer's ID for a slirp netdev, which can be
> queried from the guest through the Get Version ID NC-SI command. For
> example, by setting the manufacturer's ID to 0x8119:
>
>     wget https://github.com/facebook/openbmc/releases/download/openbmc-e2294ff5d31d/fby35.mtd
>     qemu-system-arm -machine fby35-bmc \
>         -drive file=fby35.mtd,format=raw,if=mtd -nographic \
>         -netdev user,id=nic,mfr-id=0x8119,hostfwd=::2222-:22 \
>         -net nic,model=ftgmac100,netdev=nic
>     ...
>     username: root
>     password: 0penBmc
>     ...
>     root@bmc-oob:~# ncsi-util 0x15
>     NC-SI Command Response:
>     cmd: GET_VERSION_ID(0x15)
>     Response: COMMAND_COMPLETED(0x0000)  Reason: NO_ERROR(0x0000)
>     Payload length = 40
>
>     20: 0xf1 0xf0 0xf0 0x00
>     24: 0x00 0x00 0x00 0x00
>     28: 0x00 0x00 0x00 0x00
>     32: 0x00 0x00 0x00 0x00
>     36: 0x00 0x00 0x00 0x00
>     40: 0x00 0x00 0x00 0x00
>     44: 0x00 0x00 0x00 0x00
>     48: 0x00 0x00 0x00 0x00
>     52: 0x00 0x00 0x81 0x19
>
> Signed-off-by: Peter Delevoryas <pdel@fb.com>
> ---

[...]

> diff --git a/qapi/net.json b/qapi/net.json
> index d6f7cfd4d6..efc5cb3fb6 100644
> --- a/qapi/net.json
> +++ b/qapi/net.json
> @@ -167,6 +167,8 @@
>  #
>  # @tftp-server-name: RFC2132 "TFTP server name" string (Since 3.1)
>  #
> +# @mfr-id: Manufacturer ID (Private Enterprise Number: IANA)
> +#

Is 'mfr-id' an established technical term, or an abbreviation you came
up with?  If the latter, please use @manufacturer-id instead.

Documentation is rather terse.  It basically provides a bunch of
keywords you can throw at the search engine of your choice.  Can we cut
out that middle man and point straight to a suitable resource?

>  # Since: 1.2
>  ##
>  { 'struct': 'NetdevUserOptions',
> @@ -192,7 +194,8 @@
>      '*smbserver': 'str',
>      '*hostfwd':   ['String'],
>      '*guestfwd':  ['String'],
> -    '*tftp-server-name': 'str' } }
> +    '*tftp-server-name': 'str',
> +    '*mfr-id': 'uint32' } }
>  
>  ##
>  # @NetdevTapOptions:

Re: [PATCH 3/4] slirp: Add mfr-id to -netdev options

[Peter Delevoryas]

[-- Attachment #1: Type: text/plain, Size: 3206 bytes --]



On Jun 20, 2022, at 12:16 AM, Markus Armbruster <armbru@redhat.com<mailto:armbru@redhat.com>> wrote:

Peter Delevoryas <pdel@fb.com<mailto:pdel@fb.com>> writes:

This lets you set the manufacturer's ID for a slirp netdev, which can be
queried from the guest through the Get Version ID NC-SI command. For
example, by setting the manufacturer's ID to 0x8119:

   wget https://github.com/facebook/openbmc/releases/download/openbmc-e2294ff5d31d/fby35.mtd
   qemu-system-arm -machine fby35-bmc \
       -drive file=fby35.mtd,format=raw,if=mtd -nographic \
       -netdev user,id=nic,mfr-id=0x8119,hostfwd=::2222-:22 \
       -net nic,model=ftgmac100,netdev=nic
   ...
   username: root
   password: 0penBmc
   ...
   root@bmc-oob:~# ncsi-util 0x15
   NC-SI Command Response:
   cmd: GET_VERSION_ID(0x15)
   Response: COMMAND_COMPLETED(0x0000)  Reason: NO_ERROR(0x0000)
   Payload length = 40

   20: 0xf1 0xf0 0xf0 0x00
   24: 0x00 0x00 0x00 0x00
   28: 0x00 0x00 0x00 0x00
   32: 0x00 0x00 0x00 0x00
   36: 0x00 0x00 0x00 0x00
   40: 0x00 0x00 0x00 0x00
   44: 0x00 0x00 0x00 0x00
   48: 0x00 0x00 0x00 0x00
   52: 0x00 0x00 0x81 0x19

Signed-off-by: Peter Delevoryas <pdel@fb.com<mailto:pdel@fb.com>>
---

[...]

diff --git a/qapi/net.json b/qapi/net.json
index d6f7cfd4d6..efc5cb3fb6 100644
--- a/qapi/net.json
+++ b/qapi/net.json
@@ -167,6 +167,8 @@
#
# @tftp-server-name: RFC2132 "TFTP server name" string (Since 3.1)
#
+# @mfr-id: Manufacturer ID (Private Enterprise Number: IANA)
+#

Is 'mfr-id' an established technical term, or an abbreviation you came
up with?  If the latter, please use @manufacturer-id instead.

I don’t think so, I used that particular variable name throughout these patches
because that’s what the Linux kernel driver was using, but you’re right, we should
not use an abbreviation, in v2 I’ll change it to @manufacturer-id.


Documentation is rather terse.  It basically provides a bunch of
keywords you can throw at the search engine of your choice.  Can we cut
out that middle man and point straight to a suitable resource?

Erg, yeah, sorry about that, you’re right, it would probably be more useful
to point to the NC-SI specification directly:

https://www.dmtf.org/sites/default/files/standards/documents/DSP0222_1.0.0.pdf

Note: there have been some newer revisions to the specification lately, and the
full list of spec versions is here:

https://www.dmtf.org/dsp/DSP0222

Get Version ID and the OEM Vendor extension are both specified in 1.0.0, so I
think it should be ok to link to 1.0.0.

I’m not totally sure if I should directly link to the actual URL, but I’ll
definitely say: “This is defined in DMTF NC-SI 1.0.0” or something like that.
Unless URL’s in the code would be preferred. Theoretically, the DMTF
spec URL should be pretty long-lasting.

Thanks,
Peter


# Since: 1.2
##
{ 'struct': 'NetdevUserOptions',
@@ -192,7 +194,8 @@
    '*smbserver': 'str',
    '*hostfwd':   ['String'],
    '*guestfwd':  ['String'],
-    '*tftp-server-name': 'str' } }
+    '*tftp-server-name': 'str',
+    '*mfr-id': 'uint32' } }

##
# @NetdevTapOptions:



[-- Attachment #2: Type: text/html, Size: 6177 bytes --]

Re: [PATCH 2/4] slirp: Update SlirpConfig version to 5

[Peter Delevoryas]

> On Jun 18, 2022, at 3:03 AM, Samuel Thibault <samuel.thibault@gnu.org> wrote:
> 
> Hello,
> 
> Peter Delevoryas, le mer. 15 juin 2022 18:05:24 -0700, a ecrit:
>> I think we probably need a new Slirp release
>> (4.8.0) and a switch statement here instead, right?
>> 
>> So that we can preserve the behavior for 4.7.0?
> 
> Yes, that's the idea.

Ok great, thanks!

> 
> Samuel

Re: [PATCH 3/4] slirp: Add mfr-id to -netdev options

[Peter Delevoryas]

> On Jun 18, 2022, at 3:05 AM, Samuel Thibault <samuel.thibault@gnu.org> wrote:
> 
> Peter Delevoryas, le mer. 15 juin 2022 18:05:25 -0700, a ecrit:
>> This lets you set the manufacturer's ID for a slirp netdev, which can be
>> queried from the guest through the Get Version ID NC-SI command. For
>> example, by setting the manufacturer's ID to 0x8119:
>> 
>> wget https://github.com/facebook/openbmc/releases/download/openbmc-e2294ff5d31d/fby35.mtd
>> qemu-system-arm -machine fby35-bmc \
>> -drive file=fby35.mtd,format=raw,if=mtd -nographic \
>> -netdev user,id=nic,mfr-id=0x8119,hostfwd=::2222-:22 \
>> -net nic,model=ftgmac100,netdev=nic
>> ...
>> username: root
>> password: 0penBmc
>> ...
>> root@bmc-oob:~# ncsi-util 0x15
>> NC-SI Command Response:
>> cmd: GET_VERSION_ID(0x15)
>> Response: COMMAND_COMPLETED(0x0000) Reason: NO_ERROR(0x0000)
>> Payload length = 40
>> 
>> 20: 0xf1 0xf0 0xf0 0x00
>> 24: 0x00 0x00 0x00 0x00
>> 28: 0x00 0x00 0x00 0x00
>> 32: 0x00 0x00 0x00 0x00
>> 36: 0x00 0x00 0x00 0x00
>> 40: 0x00 0x00 0x00 0x00
>> 44: 0x00 0x00 0x00 0x00
>> 48: 0x00 0x00 0x00 0x00
>> 52: 0x00 0x00 0x81 0x19
>> 
>> Signed-off-by: Peter Delevoryas <pdel@fb.com>
>> ---
>> net/slirp.c | 5 +++--
>> qapi/net.json | 5 ++++-
>> 2 files changed, 7 insertions(+), 3 deletions(-)
>> 
>> diff --git a/net/slirp.c b/net/slirp.c
>> index 75e5ccafd9..231068c1e2 100644
>> --- a/net/slirp.c
>> +++ b/net/slirp.c
>> @@ -413,7 +413,7 @@ static int net_slirp_init(NetClientState *peer, const char *model,
>> const char *vnameserver, const char *vnameserver6,
>> const char *smb_export, const char *vsmbserver,
>> const char **dnssearch, const char *vdomainname,
>> - const char *tftp_server_name,
>> + const char *tftp_server_name, uint32_t mfr_id,
>> Error **errp)
>> {
>> /* default settings according to historic slirp */
>> @@ -636,6 +636,7 @@ static int net_slirp_init(NetClientState *peer, const char *model,
>> cfg.vnameserver6 = ip6_dns;
>> cfg.vdnssearch = dnssearch;
>> cfg.vdomainname = vdomainname;
>> + cfg.mfr_id = mfr_id;
> 
> You will need a #if to only include it with slirp 4.8.0 indeed.
> Otherwise the mfr_id field won't exist.
> 

Oh, right. I’ll test this code with 4.7.0 and 4.8.0 and make
sure the behavior is correct (and perhaps even with earlier
versions like 1.0).

> In the #else part, it would probably useful to give the user at least a
> warning that tells her to upgrade slirp to 4.8.0 to get the mfr_id
> functionality working.

Ah yes good idea, I’ll include that.

> 
>> s->slirp = slirp_new(&cfg, &slirp_cb, s);
>> QTAILQ_INSERT_TAIL(&slirp_stacks, s, entry);
>> 
>> @@ -1172,7 +1173,7 @@ int net_init_slirp(const Netdev *netdev, const char *name,
>> user->bootfile, user->dhcpstart,
>> user->dns, user->ipv6_dns, user->smb,
>> user->smbserver, dnssearch, user->domainname,
>> - user->tftp_server_name, errp);
>> + user->tftp_server_name, user->mfr_id, errp);
>> 
>> while (slirp_configs) {
>> config = slirp_configs;
>> diff --git a/qapi/net.json b/qapi/net.json
>> index d6f7cfd4d6..efc5cb3fb6 100644
>> --- a/qapi/net.json
>> +++ b/qapi/net.json
>> @@ -167,6 +167,8 @@
>> #
>> # @tftp-server-name: RFC2132 "TFTP server name" string (Since 3.1)
>> #
>> +# @mfr-id: Manufacturer ID (Private Enterprise Number: IANA)
>> +#
>> # Since: 1.2
>> ##
>> { 'struct': 'NetdevUserOptions',
>> @@ -192,7 +194,8 @@
>> '*smbserver': 'str',
>> '*hostfwd': ['String'],
>> '*guestfwd': ['String'],
>> - '*tftp-server-name': 'str' } }
>> + '*tftp-server-name': 'str',
>> + '*mfr-id': 'uint32' } }
>> 
>> ##
>> # @NetdevTapOptions:
>> -- 
>> 2.30.2

Re: [PATCH 4/4] slirp: Add oob-eth-addr to -netdev options

[Peter Delevoryas]

> On Jun 18, 2022, at 3:05 AM, Samuel Thibault <samuel.thibault@gnu.org> wrote:
> 
> Peter Delevoryas, le mer. 15 juin 2022 18:05:26 -0700, a ecrit:
>> With this change, you can now request the out-of-band MAC address from
>> slirp in fby35-bmc:
>> 
>>    wget https://github.com/facebook/openbmc/releases/download/openbmc-e2294ff5d31d/fby35.mtd
>>    qemu-system-arm -machine fby35-bmc \
>>        -drive file=fby35.mtd,format=raw,if=mtd \
>>        -nographic \
>>        -netdev user,id=nic,mfr-id=0x8119,oob-eth-addr=de:ad:be:ef:ca:fe,hostfwd=::2222-:22 \
>>        -net nic,model=ftgmac100,netdev=nic
>> 
>>    ...
>>    username: root
>>    password: 0penBmc
>>    ...
>> 
>>    root@bmc-oob:~# ncsi-util -n eth0 -c 0 0x50 0 0 0x81 0x19 0 0 0x1b 0
>>    NC-SI Command Response:
>>    cmd: NCSI_OEM_CMD(0x50)
>>    Response: COMMAND_COMPLETED(0x0000)  Reason: NO_ERROR(0x0000)
>>    Payload length = 24
>> 
>>    20: 0x00 0x00 0x81 0x19
>>    24: 0x01 0x00 0x1b 0x00
>>    28: 0x00 0x00 0x00 0x00
>>    32: 0xde 0xad 0xbe 0xef
>>    36: 0xca 0xfe 0x00 0x00
>> 
>>    root@bmc-oob:~# ifconfig
>>    eth0      Link encap:Ethernet  HWaddr DE:AD:BE:EF:CA:FE
>> 	      inet addr:10.0.2.15  Bcast:10.0.2.255  Mask:255.255.255.0
>> 	      inet6 addr: fec0::dcad:beff:feef:cafe/64 Scope:Site
>> 	      inet6 addr: fe80::dcad:beff:feef:cafe/64 Scope:Link
>> 	      UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
>> 	      RX packets:253 errors:0 dropped:0 overruns:0 frame:0
>> 	      TX packets:271 errors:0 dropped:0 overruns:0 carrier:0
>> 	      collisions:0 txqueuelen:1000
>> 	      RX bytes:24638 (24.0 KiB)  TX bytes:18876 (18.4 KiB)
>> 	      Interrupt:32
>> 
>>    lo        Link encap:Local Loopback
>> 	      inet addr:127.0.0.1  Mask:255.0.0.0
>> 	      inet6 addr: ::1/128 Scope:Host
>> 	      UP LOOPBACK RUNNING  MTU:65536  Metric:1
>> 	      RX packets:2 errors:0 dropped:0 overruns:0 frame:0
>> 	      TX packets:2 errors:0 dropped:0 overruns:0 carrier:0
>> 	      collisions:0 txqueuelen:1000
>> 	      RX bytes:120 (120.0 B)  TX bytes:120 (120.0 B)
>> 
>> Signed-off-by: Peter Delevoryas <pdel@fb.com>
>> ---
>> net/slirp.c   | 13 +++++++++++--
>> qapi/net.json |  5 ++++-
>> 2 files changed, 15 insertions(+), 3 deletions(-)
>> 
>> diff --git a/net/slirp.c b/net/slirp.c
>> index 231068c1e2..858d3da859 100644
>> --- a/net/slirp.c
>> +++ b/net/slirp.c
>> @@ -414,7 +414,7 @@ static int net_slirp_init(NetClientState *peer, const char *model,
>>                           const char *smb_export, const char *vsmbserver,
>>                           const char **dnssearch, const char *vdomainname,
>>                           const char *tftp_server_name, uint32_t mfr_id,
>> -                          Error **errp)
>> +                          uint8_t oob_eth_addr[ETH_ALEN], Error **errp)
>> {
>>     /* default settings according to historic slirp */
>>     struct in_addr net  = { .s_addr = htonl(0x0a000200) }; /* 10.0.2.0 */
>> @@ -637,6 +637,7 @@ static int net_slirp_init(NetClientState *peer, const char *model,
>>     cfg.vdnssearch = dnssearch;
>>     cfg.vdomainname = vdomainname;
>>     cfg.mfr_id = mfr_id;
>> +    memcpy(cfg.oob_eth_addr, oob_eth_addr, ETH_ALEN);
> 
> And similarly here.

Copy that, thanks again for the review comments!

> 
>>     s->slirp = slirp_new(&cfg, &slirp_cb, s);
>>     QTAILQ_INSERT_TAIL(&slirp_stacks, s, entry);
>> 
>> @@ -1142,6 +1143,7 @@ int net_init_slirp(const Netdev *netdev, const char *name,
>>     const NetdevUserOptions *user;
>>     const char **dnssearch;
>>     bool ipv4 = true, ipv6 = true;
>> +    MACAddr oob_eth_addr = {};
>> 
>>     assert(netdev->type == NET_CLIENT_DRIVER_USER);
>>     user = &netdev->u.user;
>> @@ -1166,6 +1168,12 @@ int net_init_slirp(const Netdev *netdev, const char *name,
>>     net_init_slirp_configs(user->hostfwd, SLIRP_CFG_HOSTFWD);
>>     net_init_slirp_configs(user->guestfwd, 0);
>> 
>> +    if (user->has_oob_eth_addr &&
>> +        net_parse_macaddr(oob_eth_addr.a, user->oob_eth_addr) < 0) {
>> +        error_setg(errp, "invalid syntax for OOB ethernet address");
>> +        return -1;
>> +    }
>> +
>>     ret = net_slirp_init(peer, "user", name, user->q_restrict,
>>                          ipv4, vnet, user->host,
>>                          ipv6, user->ipv6_prefix, user->ipv6_prefixlen,
>> @@ -1173,7 +1181,8 @@ int net_init_slirp(const Netdev *netdev, const char *name,
>>                          user->bootfile, user->dhcpstart,
>>                          user->dns, user->ipv6_dns, user->smb,
>>                          user->smbserver, dnssearch, user->domainname,
>> -                         user->tftp_server_name, user->mfr_id, errp);
>> +                         user->tftp_server_name, user->mfr_id, oob_eth_addr.a,
>> +                         errp);
>> 
>>     while (slirp_configs) {
>>         config = slirp_configs;
>> diff --git a/qapi/net.json b/qapi/net.json
>> index efc5cb3fb6..7b2c3c205c 100644
>> --- a/qapi/net.json
>> +++ b/qapi/net.json
>> @@ -169,6 +169,8 @@
>> #
>> # @mfr-id: Manufacturer ID (Private Enterprise Number: IANA)
>> #
>> +# @oob-eth-addr: Out-of-band ethernet address for BMC
>> +#
>> # Since: 1.2
>> ##
>> { 'struct': 'NetdevUserOptions',
>> @@ -195,7 +197,8 @@
>>     '*hostfwd':   ['String'],
>>     '*guestfwd':  ['String'],
>>     '*tftp-server-name': 'str',
>> -    '*mfr-id': 'uint32' } }
>> +    '*mfr-id': 'uint32',
>> +    '*oob-eth-addr': 'str' } }
>> 
>> ##
>> # @NetdevTapOptions:
>> -- 
>> 2.30.2
>> 
> 
> -- 
> Samuel
> ---
> Pour une évaluation indépendante, transparente et rigoureuse !
> Je soutiens la Commission d'Évaluation de l'Inria.

Re: [PATCH 3/4] slirp: Add mfr-id to -netdev options

[Markus Armbruster]

Peter Delevoryas <pdel@fb.com> writes:

> On Jun 20, 2022, at 12:16 AM, Markus Armbruster <armbru@redhat.com<mailto:armbru@redhat.com>> wrote:

[...]

>> Documentation is rather terse.  It basically provides a bunch of
>> keywords you can throw at the search engine of your choice.  Can we cut
>> out that middle man and point straight to a suitable resource?
>
> Erg, yeah, sorry about that, you’re right, it would probably be more useful
> to point to the NC-SI specification directly:
>
> https://www.dmtf.org/sites/default/files/standards/documents/DSP0222_1.0.0.pdf
>
> Note: there have been some newer revisions to the specification lately, and the
> full list of spec versions is here:
>
> https://www.dmtf.org/dsp/DSP0222
>
> Get Version ID and the OEM Vendor extension are both specified in 1.0.0, so I
> think it should be ok to link to 1.0.0.
>
> I’m not totally sure if I should directly link to the actual URL, but I’ll
> definitely say: “This is defined in DMTF NC-SI 1.0.0” or something like that.

Works for me with the full name of the spec: "DMTF Network Controller
Sideband Interface (NC-SI) Specification, version 1.0.0".  You can omit
the version if there are no earlier ones.

> Unless URL’s in the code would be preferred. Theoretically, the DMTF
> spec URL should be pretty long-lasting.

Feel free to add URL, too.