Attention is currently required from: osmith, laforge. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/31025 )

Change subject: Introduce tundev API ......................................................................

Patch Set 11:

(10 comments)

File include/osmocom/core/tun.h:

https://gerrit.osmocom.org/c/libosmocore/+/31025/comment/7f40d1e2_79c58aba PS9, Line 29: const char *osmo_tundev_get_name(const struct osmo_tundev *tundev);

maybe add a new line below this? given that the two functions below are about dev_name, and there's […]

Ack

File src/core/tun.c:

https://gerrit.osmocom.org/c/libosmocore/+/31025/comment/983491c8_0cbf25d9 PS4, Line 421: osmo_tundev_encaps

agreed

Ack

File src/core/tun.c:

https://gerrit.osmocom.org/c/libosmocore/+/31025/comment/af32b144_9e7c3eb7 PS9, Line 130: "

Add "Prepare netns: " for consistency with other messages in this function?

Ack

https://gerrit.osmocom.org/c/libosmocore/+/31025/comment/4bb8814e_2817ac8d PS9, Line 184: NULL

use a global talloc ctx and pass that here?

I don't plan to pass this from the API, since the ctx passed there is per tundev and may be expected to be freed when the parent object in the app holding the tudnev is released. We could add an internal static talloc ctx here if needed, but I don't see urgent need for it?

https://gerrit.osmocom.org/c/libosmocore/+/31025/comment/1c903b80_6b7d7fb1 PS9, Line 233: inreface

interface

Ack

https://gerrit.osmocom.org/c/libosmocore/+/31025/comment/fbe9b047_e0eb751e PS9, Line 284: tundev_mnl_link_state_change

Maybe call it "tundev_mnl_link_on_state_change" (adding _on_)? […]

Ack

https://gerrit.osmocom.org/c/libosmocore/+/31025/comment/8a34baba_4e28637f PS9, Line 371: |=

is this intentionally the same as in the if (up) case? if so, why have it in both the if and else br […]

Yes this is expected, as per https://git.netfilter.org/libmnl/tree/examples/rtnl/rtnl-link-set.c

https://gerrit.osmocom.org/c/libosmocore/+/31025/comment/32b65316_5fab012c PS9, Line 375: /* RTM_NEWLINK doesn't work either */

leftover comment?

Ack

https://gerrit.osmocom.org/c/libosmocore/+/31025/comment/409a82f6_83285fbe PS9, Line 448: status

why status and not len?

Ack

https://gerrit.osmocom.org/c/libosmocore/+/31025/comment/504182d4_eec11582 PS9, Line 524: free

open

Ack