Attention is currently required from: its_Giaan, plaisthos. flichtenheld has posted comments on this change by its_Giaan. ( http://gerrit.openvpn.net/c/openvpn/+/1521?usp=email )
Change subject: Add support for user defined network namespace ...................................................................... Patch Set 1: Code-Review-2 (26 comments) Patchset: PS1: I stopped reviewing at some point. Enough to fix for now File src/openvpn/dco.c: http://gerrit.openvpn.net/c/openvpn/+/1521/comment/f5b745f0_c74ba00a?usp=email : PS1, Line 394: msg(msglevel, "Note: --netns not supported by ovpn-dco, disabling data channel offload."); `ovpn` ? Or just "DCO"? File src/openvpn/networking_sitnl.h: http://gerrit.openvpn.net/c/openvpn/+/1521/comment/fa0e61b1_09e3ce72?usp=email : PS1, Line 52: * NETNS_RUN_DIR, e.g. /var/run/netns/<name>). Doxygen says `networking_sitnl.h:52: warning: Unsupported xml/html tag <name> found` http://gerrit.openvpn.net/c/openvpn/+/1521/comment/1e561559_8521c63f?usp=email : PS1, Line 98: int openvpn_if_nametoindex(const char *ifname, int netnsid); This is ALWAYS called with `get_or_create_netnsid_sitnl`, so just move all of that code inside and change the signature. File src/openvpn/networking_sitnl.c: http://gerrit.openvpn.net/c/openvpn/+/1521/comment/97ff67c1_781261b5?usp=email : PS1, Line 74: #define NETNS_RUN_DIR "/var/run/netns" Wouldn't `/run` be a better value here? `/var/run` is just a symlink nowadays AFAIK http://gerrit.openvpn.net/c/openvpn/+/1521/comment/289f0216_601a33c5?usp=email : PS1, Line 206: msg(M_WARN, "%s: Network namespace %s does not exist\n", __func__, path); M_ERRNO? http://gerrit.openvpn.net/c/openvpn/+/1521/comment/d1511ce1_4a61a209?usp=email : PS1, Line 239: msg(M_WARN, "%s: Cannot open original network namespace\n", __func__); M_ERRNO? http://gerrit.openvpn.net/c/openvpn/+/1521/comment/db2ee770_077ea5de?usp=email : PS1, Line 247: msg(M_WARN, "%s: Cannot open network namespace \"%s\"\n", __func__, name); M_ERRNO http://gerrit.openvpn.net/c/openvpn/+/1521/comment/60e6c6bb_45c54b89?usp=email : PS1, Line 254: msg(M_WARN, "%s: setting the network namespace \"%s\" failed\n", __func__, name); M_ERRNO http://gerrit.openvpn.net/c/openvpn/+/1521/comment/5e711262_50c89506?usp=email : PS1, Line 272: msg(M_WARN, "%s: Cannot restore original network namespace\n", __func__); M_ERRNO http://gerrit.openvpn.net/c/openvpn/+/1521/comment/15d5672a_5f164ecd?usp=email : PS1, Line 635: goto err; just use return http://gerrit.openvpn.net/c/openvpn/+/1521/comment/e3edca05_87e906c4?usp=email : PS1, Line 841: goto err; M_ERR is fatal http://gerrit.openvpn.net/c/openvpn/+/1521/comment/5ed22de4_b4c7950e?usp=email : PS1, Line 848: goto err; M_ERR is fatal http://gerrit.openvpn.net/c/openvpn/+/1521/comment/7cb5d904_f6b98d2c?usp=email : PS1, Line 914: if (netns) These 4 lines (counting the netns assignment) are copied often enough that they should be moved to a function or macro. http://gerrit.openvpn.net/c/openvpn/+/1521/comment/48e4eb65_4e5b7e66?usp=email : PS1, Line 928: msg(M_WARN, "%s: rtnl: cannot get ifindex for %s: %s", __func__, iface, strerror(errno)); Should use M_ERRNO instead of manually using strerror http://gerrit.openvpn.net/c/openvpn/+/1521/comment/e1d31283_33bdeb60?usp=email : PS1, Line 950: ret = sitnl_send(&req.n, 0, 0, NULL, NULL); Just declare ret here. C has moved on... http://gerrit.openvpn.net/c/openvpn/+/1521/comment/f52dde96_e5b369fd?usp=email : PS1, Line 953: goto err; Just use return http://gerrit.openvpn.net/c/openvpn/+/1521/comment/e4633213_a3d2a888?usp=email : PS1, Line 955: if (orig >= 0) Also move to function or macro http://gerrit.openvpn.net/c/openvpn/+/1521/comment/3706333e_7b657959?usp=email : PS1, Line 1002: err: Unused label? http://gerrit.openvpn.net/c/openvpn/+/1521/comment/a2a79f77_a7e3716c?usp=email : PS1, Line 1019: spurious empty line http://gerrit.openvpn.net/c/openvpn/+/1521/comment/99000be3_9e6be45b?usp=email : PS1, Line 1064: spurious empty line http://gerrit.openvpn.net/c/openvpn/+/1521/comment/7c543e33_d1045522?usp=email : PS1, Line 1124: err: Unused label http://gerrit.openvpn.net/c/openvpn/+/1521/comment/a9d4fe8c_8da527ff?usp=email : PS1, Line 1833: goto err; WHAT? http://gerrit.openvpn.net/c/openvpn/+/1521/comment/57c51c0b_69ab4bb4?usp=email : PS1, Line 1881: goto err; lol http://gerrit.openvpn.net/c/openvpn/+/1521/comment/09b54618_34113780?usp=email : PS1, Line 1903: return netnsid; Hmm, why does clang-format not fix this? File src/openvpn/options.c: http://gerrit.openvpn.net/c/openvpn/+/1521/comment/cbd845cd_a862dec9?usp=email : PS1, Line 6476: #if defined(TARGET_LINUX) The documentation says it is a no-op on non-Linux, but as you implemented it is an error I think? -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1521?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: I8b0d1cad7062856abcc40c4e16ec93b45295bbd3 Gerrit-Change-Number: 1521 Gerrit-PatchSet: 1 Gerrit-Owner: its_Giaan <[email protected]> Gerrit-Reviewer: flichtenheld <[email protected]> Gerrit-Reviewer: plaisthos <[email protected]> Gerrit-CC: openvpn-devel <[email protected]> Gerrit-Attention: plaisthos <[email protected]> Gerrit-Attention: its_Giaan <[email protected]> Gerrit-Comment-Date: Mon, 16 Feb 2026 12:10:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes
_______________________________________________ Openvpn-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/openvpn-devel
