network: specify netlink protocol

[bergwolf]

The go netlink package has added more netlink socket protocols
than we need. Specify one to avoid failure due to unsupported
protocols.

Fixes: #209

[codecov]

Codecov Report

Merging #210 into master will increase coverage by 0.67%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master     #210      +/-   ##
==========================================
+ Coverage    34.7%   35.37%   +0.67%     
==========================================
  Files          11       11              
  Lines        1533     1631      +98     
==========================================
+ Hits          532      577      +45     
- Misses        929      969      +40     
- Partials       72       85      +13

Impacted Files	Coverage Δ
network.go	46.24% <0%> (-0.21%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a80ad16...131554d. Read the comment docs.

[nitkon]

LGTM. I tested it out on my x86 machine and the kata container is getting created with the hyperstart kernel using the initrd approach.

sudo docker run -ti --runtime kata-runtime busybox sh
Unable to find image 'busybox:latest' locally
latest: Pulling from library/busybox
f70adabe43c0: Already exists
Digest: sha256:58ac43b2cc92c687a32c8be6278e50a063579655fe3090125dcb2af0ff9e1a64
Status: Downloaded newer image for busybox:latest
/ # ls
bin dev etc home proc root sys tmp usr var

[devimc]

@bergwolf does this change is to remove XFRM support from the kernel?

[bergwolf]

@devimc actually it was NETLINK_NETFILTER that caused the docker failure.

[sboeuf]

@grahamwhaley PTAL.

[devimc]

@bergwolf ok, then IP_SET is not needed, right?

[sboeuf]

@devimc could you give more context about what you're talking about ? I am asking because I don't understand the relation between your comments and this change.

[devimc]

@sboeuf this change impacts to the kernel, when a new netlink handler is created three netlink families are used, NETLINK_ROUTE, NETLINK_XFRM and NETLINK_NETFILTER, that means kernel need to have enabled these three families, if this patch is merged, then we can rid of NETLINK_XFRM and NETLINK_NETFILTER, since we don't need them

[devimc]

@bergwolf do you know what config do we need to enable NETLINK_ROUTE ?

[sboeuf]

@devimc thanks for the explanation here. So because this change makes only use of NETLINK_ROUTE, you're trying to reduce our Kata Containers kernel.

[devimc]

@sboeuf yes

[sboeuf]

@devimc Ok then I think we could open an issue for that, as I expect that we will merge this PR very soon. This way we can continue the discussion.

[devimc]

@sboeuf kata-containers/linux#5 has not been merged, so I'll update the PR

[bergwolf]

@devimc We are apparently not depending on iptables for setting up the guest filtering rules. So it looks like NETLINK_NETFILTER (and IP_SET that depends on it) is not required at the moment. But will we depend on iptables at some point in the future?

[devimc]

@bergwolf good question, @mcastelino @amshinde WDYT ?

[mcastelino]

@devimc we can wait till we see the need for NETLINK_NETFILTER.

@bergwolf have you run into a situation where you have set iptable rules inside the VM. I noticed in your runv/frakti code do had a option to setup port forwarding rules inside the VM? When did you use this?

[amshinde]

lgtm

[amshinde]

@mcastelino We may end up needing iptables for supporting docker compose in the future.

[bergwolf]

@mcastelino in runv, we use iptables to setup port mapping and to support docker CNM.

@ALL whether we need NETLINK_XFRM and NETLINK_NETFILTER is not relevant to this patch. Can we go ahead and merge it and have another issue to discuss kernel config options?

[sboeuf]

I am all for merging this. @egernst @grahamwhaley PTAL.
And let's continue the discussion on a separate issue or PR (here kata-containers/linux#5 as indicated by @devimc).

[grahamwhaley]

change
lgtm
the discussion about kernel configs can happen elsewhere indeed.
codecov is not too happy - I think mainly because there is not huge test coverage for this file - but, these changes don't really make it any worse, so should not be a hard block on a merge (but, it would be nice if we could sneak some more tests in at the same time ;-) )

[devimc]

lgtm

[bergwolf]

agreed @grahamwhaley. Let's merge ignoring the codecov failure (which actually reports increase coverage by 0.67%.).