Hi!
07.12.2020, 08:52, "Jason Wang" <jasow...@redhat.com>:
> On 2020/11/9 上午7:59, Alexey Kirillov wrote:
>> +#ifdef CONFIG_SLIRP
>> + case NET_BACKEND_USER: {
>> + size_t len = strchr(ni->u.user.net, '/') - ni->u.user.net;
>> + char *net = g_strndup(ni->u.user.net, len);
>> +
>> + info_str = g_strdup_printf("net=%s,restrict=%s",
>> + net,
>> + ni->u.user.q_restrict ? "on" : "off");
>> + g_free(net);
>> + break;
>> + }
>> +#endif /* CONFIG_SLIRP */
>> + case NET_BACKEND_TAP: {
>> +#ifndef _WIN32
>> + if (ni->u.tap.has_fds) {
>> + char **fds = g_strsplit(ni->u.tap.fds, ":", -1);
>> +
>> + info_str = g_strdup_printf("fd=%s", fds[nc->queue_index]);
>> + g_strfreev(fds);
>> + } else if (ni->u.tap.has_helper) {
>> + info_str = g_strdup_printf("helper=%s", ni->u.tap.helper);
>> + } else {
>> + info_str = g_strdup_printf("ifname=%s,script=%s,downscript=%s",
>> + ni->u.tap.ifname,
>> + nc->queue_index == 0 ? ni->u.tap.script : "no",
>> + nc->queue_index == 0 ? ni->u.tap.downscript : "no");
>> + }
>> +#else
>> + info_str = g_strdup_printf("tap: ifname=%s", ni->u.tap.ifname);
>> +#endif /* _WIN32 */
>> + break;
>> + }
>> +#ifdef CONFIG_L2TPV3
>> + case NET_BACKEND_L2TPV3: {
>> + info_str = g_strdup_printf("l2tpv3: connected");
>> + break;
>> + }
>> +#endif /* CONFIG_L2TPV3 */
>> + case NET_BACKEND_SOCKET: {
>> + if (ni->u.socket.has_listen) {
>> + if (ni->u.socket.has_fd) {
>> + info_str = g_strdup_printf("socket: connection from %s",
>> + ni->u.socket.listen);
>> + } else {
>> + info_str = g_strdup_printf("socket: wait from %s",
>> + ni->u.socket.listen);
>> + }
>> + } else if (ni->u.socket.has_connect && ni->u.socket.has_fd) {
>> + info_str = g_strdup_printf("socket: connect to %s",
>> + ni->u.socket.connect);
>> + } else if (ni->u.socket.has_mcast && ni->u.socket.has_fd) {
>> + info_str = g_strdup_printf("socket: mcast=%s",
>> + ni->u.socket.mcast);
>> + } else if (ni->u.socket.has_udp && ni->u.socket.has_fd) {
>> + info_str = g_strdup_printf("socket: udp=%s", ni->u.socket.udp);
>> + } else {
>> + g_assert(ni->u.socket.has_fd);
>> + int so_type = -1;
>> + int optlen = sizeof(so_type);
>> + int fd = atoi(ni->u.socket.fd);
>> +
>> + getsockopt(fd, SOL_SOCKET, SO_TYPE, (char *)&so_type,
>> + (socklen_t *)&optlen);
>> + if (so_type == SOCK_STREAM) {
>> + info_str = g_strdup_printf("socket: fd=%s",
>> + ni->u.socket.fd);
>> + } else {
>> + if (ni->u.socket.has_mcast) {
>> + /*
>> + * This branch is unreachable, according to how it is in
>> + * net/socket.c at this moment
>> + */
>> + info_str = g_strdup_printf("socket: fd=%s "
>> + "(cloned mcast=%s)",
>> + ni->u.socket.fd,
>> + ni->u.socket.mcast);
>> + } else {
>> + SocketAddress *sa = socket_local_address(fd, NULL);
>> +
>> + info_str = g_strdup_printf("socket: fd=%s %s",
>> + ni->u.socket.fd,
>> + SocketAddressType_str(sa->type));
>> + qapi_free_SocketAddress(sa);
>> + }
>> + }
>> + }
>> + break;
>> + }
>> +#ifdef CONFIG_VDE
>> + case NET_BACKEND_VDE: {
>> + info_str = g_strdup_printf("sock=%s,fd=%d",
>> + ni->u.vde.sock,
>> + net_vde_get_fd(nc));
>> + break;
>> + }
>> +#endif /* CONFIG_VDE */
>> +#ifdef CONFIG_NET_BRIDGE
>> + case NET_BACKEND_BRIDGE: {
>> + info_str = g_strdup_printf("helper=%s,br=%s",
>> + ni->u.bridge.helper,
>> + ni->u.bridge.br);
>> + break;
>> + }
>> +#endif /* CONFIG_NET_BRIDGE */
>> +#ifdef CONFIG_NETMAP
>> + case NET_BACKEND_NETMAP: {
>> + info_str = g_strdup_printf("netmap: ifname=%s",
>> + ni->u.netmap.ifname);
>> + break;
>> + }
>> +#endif /* CONFIG_NETMAP */
>> +#ifdef CONFIG_VHOST_NET_USER
>> + case NET_BACKEND_VHOST_USER: {
>> + info_str = g_strdup_printf("vhost-user%d to %s",
>> + nc->queue_index,
>> + ni->u.vhost_user.chardev);
>> + break;
>> + }
>> +#endif /* CONFIG_VHOST_NET_USER */
>> +#ifdef CONFIG_VHOST_NET_VDPA
>> + case NET_BACKEND_VHOST_VDPA: {
>> + info_str = g_strdup("vhost-vdpa");
>> + break;
>> + }
>> +#endif /* CONFIG_VHOST_NET_VDPA */
>
> This will introduce burdens for new netdevs or new attributes since
> people can easily forget to add the routine here.
>
> I think at least we need introduce callbacks for this.
Thanks for pointing. I can't remember why exactly I chose to not do it.
So it's definitely better to split this chunk to several callbacks.
I'll do it in the next version of series.
> One more stupid question, instead of generating the string via hard
> codes, is there any method (dict?) to iterate all the key/values
> automatically?
>
> Thanks
Oh yes, that the point.
Now there are no common format for info_str.
This patch is aimed to keep old HMP command mostly untouched.
But if we can drop old format, all this mess can be generalized as JSON
lines replacing old info_str stuff.
What do you think about that?
Originally I wanted to completely drop old info_str and use
QAPI to store and provide information about netdevs (and NICs too).
Thanks!
--
Alexey Kirillov
Yandex.Cloud
