diff mbox series

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

Message ID 20220616010526.1895564-4-pdel@fb.com
State New
Headers show
Series slirp: Update submodule to include NC-SI features | expand

Commit Message

Peter Delevoryas June 16, 2022, 1:05 a.m. UTC
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(-)

Comments

Samuel Thibault June 18, 2022, 10:05 a.m. UTC | #1
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
>
Markus Armbruster June 20, 2022, 7:16 a.m. UTC | #2
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:
Peter Delevoryas June 20, 2022, 9:27 p.m. UTC | #3
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:
Peter Delevoryas June 20, 2022, 10:58 p.m. UTC | #4
> 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
Markus Armbruster June 21, 2022, 9:01 a.m. UTC | #5
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.
diff mbox series

Patch

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: