Re: [PATCH 3/4] slirp: Add mfr-id to -netdev options
Peter Delevoryas writes: > On Jun 20, 2022, at 12:16 AM, Markus Armbruster > mailto:arm...@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.
Re: [PATCH 3/4] slirp: Add mfr-id to -netdev options
> On Jun 18, 2022, at 3:05 AM, Samuel Thibault 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=::-: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(0x) Reason: NO_ERROR(0x)
>> 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
>> ---
>> 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 3/4] slirp: Add mfr-id to -netdev options
On Jun 20, 2022, at 12:16 AM, Markus Armbruster
mailto:arm...@redhat.com>> wrote:
Peter Delevoryas mailto:p...@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=::-: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(0x) Reason: NO_ERROR(0x)
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 mailto:p...@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:
Re: [PATCH 3/4] slirp: Add mfr-id to -netdev options
Peter Delevoryas 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=::-: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(0x) Reason: NO_ERROR(0x)
> 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
> ---
[...]
> 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, 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=::-: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(0x) Reason: NO_ERROR(0x)
> 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
> ---
> 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
>
