ifconfig: Add netlink helper to create interface

[pouria]

Summary

Add a helper function for netlink to
be easier to create interfaces directly using netlink.
While here, fix style issue of a line above it.

Test Plan

will be used in ifconfig_geneve and
future netlink implementations in ifconfig.

[glebius]

sbin/ifconfig/ifconfig_netlink.c

507–510

IMHO, we should errx() even if error_str is NULL.

[glebius]

Also (NETLINK) isn't a great error message at all. Please try grep errx sbin/ifconfig/*.c to check other error messages. Maybe just only contents of errmsg.error_str in the case it was provided and unknown error from netlink(4) in case it was not?

[pouria]

@glebius done.
Can I commit it?

[pouria]

After running my tests for this change, I remembered that
the netlink can reply with an NLMSG_ERROR containing error = 0.
That's why I didn't call errx() when error_str==NULL.

However, I've fixed the error message as you recommendeded. @glebius

[glebius]

In D55174#1263726, @pouria wrote:

After running my tests for this change, I remembered that
the netlink can reply with an NLMSG_ERROR containing error = 0.

That's real strange. Do you know how to reproduce that?

[pouria]

In D55174#1264254, @glebius wrote:

That's real strange. Do you know how to reproduce that?

Only with geneve, since there is no other implementation exists for now.
I prepared a branch for you to test:
https://github.com/spmzt/freebsd-src/tree/geneve_total

[glebius]

Sorry for making the Geneve check-in process longer, but I really want to understand all corner cases, instead of workarounding them.

So, can we debug how it happens that netlink can reply with an NLMSG_ERROR containing error = 0? If this is possible with Geneve only, can this came straight from the Geneve module or is it the netlink layer that produces that?

Also, should we assert or no that if hdr->nlmsg_type != NL_RTM_NEWLINK, then it is NLMSG_ERROR?

P.S. I added Alexander, might be a chance he will participate.

[pouria]

In D55174#1264547, @glebius wrote:

Sorry for making the Geneve check-in process longer, but I really want to understand all corner cases, instead of workarounding them.

I also prefer this approach. better safe than sorry

So, can we debug how it happens that netlink can reply with an NLMSG_ERROR containing error = 0? If this is possible with Geneve only, can this came straight from the Geneve module or is it the netlink layer that produces that?

I debugged it a LOT. I couldn't find out why or where it decides to return NLMSG_ERROR.
Setting the NLM_F_ACK in netlink request does not make any difference either.
Using kgdb. I set a breakpoint to rtnl_handle_message().
Then created an interface:

ifconfig geneve create genevemode l2 geneveid 10 genevelocal 192.168.1.1 geneveremote 192.168.1.2

and the hdr->nlmsg_type was always 16 which is NL_RTM_NEWLINK.
You can simply use the tests in D55183 to hit that breakpoint.

Also, should we assert or no that if hdr->nlmsg_type != NL_RTM_NEWLINK, then it is NLMSG_ERROR?

AFAICU the netlink. it should reply with either the same type that was in the request or the NLMSG_ERROR,
so I think the answer is yes. But I'm not sure. I'm not an expert in netlink.

[glebius]

sbin/ifconfig/ifconfig_netlink.c

505–511

This is how it should look like. Actually NLMSG_ERROR with error == 0 is the correct reply from netlink. See netlink_io.c:nlmsg_ack().

[pouria]

sbin/ifconfig/ifconfig_netlink.c

505–511

This is how it should look like. Actually NLMSG_ERROR with error == 0 is the correct reply from netlink. See netlink_io.c:nlmsg_ack().

Thank you. I didn't know about snl_read_reply_code. (I'll work on improving snl.3)
Now I don't see much value in implementing ifcreate_nl since it doesn't handle much code.
IMHO, now snl_send_message + snl_read_reply_code could be better directly called,
because at least ifconfig developers can handle/custom their error messages.

What do you think?
should I abandon this revision?

[glebius]

I agree that not much left of ifcreate_nl() and it can be inlined into ifgeneve.c.

For the snl(3), be aware that you are one of the very early users of this API. It is very raw and might change and improve. Actually your usage is something that may act as input for changes. So don't rush with documenting everything we have there right now rather than do some reviewing on how well it fits your needs.