| Message ID | 20201122173921.31473-1-ryazanov.s.a@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | David Ahern |
| Headers | show |
| Series | [iproute2-next] ip: add IP_LIB_DIR environment variable | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
[ sorry, this got lost in the backlog ] On 11/22/20 10:39 AM, Sergey Ryazanov wrote: > Do not hardcode /usr/lib/ip as a path and allow libraries path > configuration in run-time. > > Signed-off-by: Sergey Ryazanov <ryazanov.s.a@gmail.com> > --- > ip/ip.c | 15 +++++++++++++++ > ip/ip_common.h | 2 ++ > ip/iplink.c | 5 +---- > 3 files changed, 18 insertions(+), 4 deletions(-) > > diff --git a/ip/ip.c b/ip/ip.c > index 5e31957f..38600e51 100644 > --- a/ip/ip.c > +++ b/ip/ip.c > @@ -25,6 +25,10 @@ > #include "color.h" > #include "rt_names.h" > > +#ifndef LIBDIR > +#define LIBDIR "/usr/lib" > +#endif > + > int preferred_family = AF_UNSPEC; > int human_readable; > int use_iec; > @@ -41,6 +45,17 @@ bool do_all; > > struct rtnl_handle rth = { .fd = -1 }; > > +const char *get_ip_lib_dir(void) > +{ > + const char *lib_dir; > + > + lib_dir = getenv("IP_LIB_DIR"); > + if (!lib_dir) > + lib_dir = LIBDIR "/ip"; > + > + return lib_dir; > +} > + > static void usage(void) __attribute__((noreturn)); > > static void usage(void) > diff --git a/ip/ip_common.h b/ip/ip_common.h > index d604f755..227eddd3 100644 > --- a/ip/ip_common.h > +++ b/ip/ip_common.h > @@ -27,6 +27,8 @@ struct link_filter { > int target_nsid; > }; > > +const char *get_ip_lib_dir(void); > + > int get_operstate(const char *name); > int print_linkinfo(struct nlmsghdr *n, void *arg); > int print_addrinfo(struct nlmsghdr *n, void *arg); > diff --git a/ip/iplink.c b/ip/iplink.c > index d6b766de..4250b2c3 100644 > --- a/ip/iplink.c > +++ b/ip/iplink.c > @@ -34,9 +34,6 @@ > #include "namespace.h" > > #define IPLINK_IOCTL_COMPAT 1 > -#ifndef LIBDIR > -#define LIBDIR "/usr/lib" > -#endif > > #ifndef GSO_MAX_SIZE > #define GSO_MAX_SIZE 65536 > @@ -157,7 +154,7 @@ struct link_util *get_link_kind(const char *id) > if (strcmp(l->id, id) == 0) > return l; > > - snprintf(buf, sizeof(buf), LIBDIR "/ip/link_%s.so", id); > + snprintf(buf, sizeof(buf), "%s/link_%s.so", get_ip_lib_dir(), id); > dlh = dlopen(buf, RTLD_LAZY); > if (dlh == NULL) { > /* look in current binary, only open once */ > What's the use case for needing this? AIUI this is a legacy feature from many years ago. All of the link files are builtin, so this is only useful for out of tree modules. iproute2 should not be supporting such an option, so really this code should be ripped out, not updated.
On Mon, Nov 30, 2020 at 8:25 PM David Ahern <dsahern@gmail.com> wrote: > > [ sorry, this got lost in the backlog ] Do not worry. This patch is 1.5 years old :) I keep it in my local portages, but then I decide that it will be helpful for other developers too. > On 11/22/20 10:39 AM, Sergey Ryazanov wrote: > > Do not hardcode /usr/lib/ip as a path and allow libraries path > > configuration in run-time. > > > > Signed-off-by: Sergey Ryazanov <ryazanov.s.a@gmail.com> > > --- > > ip/ip.c | 15 +++++++++++++++ > > ip/ip_common.h | 2 ++ > > ip/iplink.c | 5 +---- > > 3 files changed, 18 insertions(+), 4 deletions(-) > > > > diff --git a/ip/ip.c b/ip/ip.c > > index 5e31957f..38600e51 100644 > > --- a/ip/ip.c > > +++ b/ip/ip.c > > @@ -25,6 +25,10 @@ > > #include "color.h" > > #include "rt_names.h" > > > > +#ifndef LIBDIR > > +#define LIBDIR "/usr/lib" > > +#endif > > + > > int preferred_family = AF_UNSPEC; > > int human_readable; > > int use_iec; > > @@ -41,6 +45,17 @@ bool do_all; > > > > struct rtnl_handle rth = { .fd = -1 }; > > > > +const char *get_ip_lib_dir(void) > > +{ > > + const char *lib_dir; > > + > > + lib_dir = getenv("IP_LIB_DIR"); > > + if (!lib_dir) > > + lib_dir = LIBDIR "/ip"; > > + > > + return lib_dir; > > +} > > + > > static void usage(void) __attribute__((noreturn)); > > > > static void usage(void) > > diff --git a/ip/ip_common.h b/ip/ip_common.h > > index d604f755..227eddd3 100644 > > --- a/ip/ip_common.h > > +++ b/ip/ip_common.h > > @@ -27,6 +27,8 @@ struct link_filter { > > int target_nsid; > > }; > > > > +const char *get_ip_lib_dir(void); > > + > > int get_operstate(const char *name); > > int print_linkinfo(struct nlmsghdr *n, void *arg); > > int print_addrinfo(struct nlmsghdr *n, void *arg); > > diff --git a/ip/iplink.c b/ip/iplink.c > > index d6b766de..4250b2c3 100644 > > --- a/ip/iplink.c > > +++ b/ip/iplink.c > > @@ -34,9 +34,6 @@ > > #include "namespace.h" > > > > #define IPLINK_IOCTL_COMPAT 1 > > -#ifndef LIBDIR > > -#define LIBDIR "/usr/lib" > > -#endif > > > > #ifndef GSO_MAX_SIZE > > #define GSO_MAX_SIZE 65536 > > @@ -157,7 +154,7 @@ struct link_util *get_link_kind(const char *id) > > if (strcmp(l->id, id) == 0) > > return l; > > > > - snprintf(buf, sizeof(buf), LIBDIR "/ip/link_%s.so", id); > > + snprintf(buf, sizeof(buf), "%s/link_%s.so", get_ip_lib_dir(), id); > > dlh = dlopen(buf, RTLD_LAZY); > > if (dlh == NULL) { > > /* look in current binary, only open once */ > > > > What's the use case for needing this? AIUI this is a legacy feature from > many years ago. > > All of the link files are builtin, so this is only useful for out of > tree modules. iproute2 should not be supporting such an option, so > really this code should be ripped out, not updated. I can not agree with your position. iproute2 is a main management utility for all network interface types, the kernel design supports new link protocols (via loadable modules), Netlink explicitly supports link management extensions, so it was a very reasonable decision to add the support for new link types to ip-link via loadable plugins. Loadable ip link plugins support is helpful at the initial project stage as well as for niche protocols, since such feature allows to introduce new protocol support without forcing users to rebuild anything. So, ripping out link plugins will make the life of many people harder without any obvious benefits for anyone else. As for the patch, it is intended to make developer life easier by allowing them to focus on development instead of maintaining a fork of iproute2 source tree. The patch introduces the same functionality to override plugins loading directory for ip, as we have in tc for a long time. Even the environment variable name was selected to be similar to that is used by tc.
On 12/1/20 3:08 AM, Sergey Ryazanov wrote: > As for the patch, it is intended to make developer life easier by > allowing them to focus on development instead of maintaining a fork of > iproute2 source tree. The patch introduces the same functionality to > override plugins loading directory for ip, as we have in tc for a long > time. Even the environment variable name was selected to be similar to > that is used by tc. > it makes ip consistent with tc, so applied to iproute2-next. Thanks
diff --git a/ip/ip.c b/ip/ip.c index 5e31957f..38600e51 100644 --- a/ip/ip.c +++ b/ip/ip.c @@ -25,6 +25,10 @@ #include "color.h" #include "rt_names.h" +#ifndef LIBDIR +#define LIBDIR "/usr/lib" +#endif + int preferred_family = AF_UNSPEC; int human_readable; int use_iec; @@ -41,6 +45,17 @@ bool do_all; struct rtnl_handle rth = { .fd = -1 }; +const char *get_ip_lib_dir(void) +{ + const char *lib_dir; + + lib_dir = getenv("IP_LIB_DIR"); + if (!lib_dir) + lib_dir = LIBDIR "/ip"; + + return lib_dir; +} + static void usage(void) __attribute__((noreturn)); static void usage(void) diff --git a/ip/ip_common.h b/ip/ip_common.h index d604f755..227eddd3 100644 --- a/ip/ip_common.h +++ b/ip/ip_common.h @@ -27,6 +27,8 @@ struct link_filter { int target_nsid; }; +const char *get_ip_lib_dir(void); + int get_operstate(const char *name); int print_linkinfo(struct nlmsghdr *n, void *arg); int print_addrinfo(struct nlmsghdr *n, void *arg); diff --git a/ip/iplink.c b/ip/iplink.c index d6b766de..4250b2c3 100644 --- a/ip/iplink.c +++ b/ip/iplink.c @@ -34,9 +34,6 @@ #include "namespace.h" #define IPLINK_IOCTL_COMPAT 1 -#ifndef LIBDIR -#define LIBDIR "/usr/lib" -#endif #ifndef GSO_MAX_SIZE #define GSO_MAX_SIZE 65536 @@ -157,7 +154,7 @@ struct link_util *get_link_kind(const char *id) if (strcmp(l->id, id) == 0) return l; - snprintf(buf, sizeof(buf), LIBDIR "/ip/link_%s.so", id); + snprintf(buf, sizeof(buf), "%s/link_%s.so", get_ip_lib_dir(), id); dlh = dlopen(buf, RTLD_LAZY); if (dlh == NULL) { /* look in current binary, only open once */
Do not hardcode /usr/lib/ip as a path and allow libraries path configuration in run-time. Signed-off-by: Sergey Ryazanov <ryazanov.s.a@gmail.com> --- ip/ip.c | 15 +++++++++++++++ ip/ip_common.h | 2 ++ ip/iplink.c | 5 +---- 3 files changed, 18 insertions(+), 4 deletions(-)