diff mbox series

[1/2] libnl: add libnl-cli library

Message ID 20210114023848.27135-1-code@simerda.eu
State Changes Requested
Headers show
Series [1/2] libnl: add libnl-cli library | expand

Commit Message

Pavel Šimerda Jan. 14, 2021, 2:38 a.m. UTC 
---
 package/libs/libnl/Makefile | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

Comments

Adrian Schmutzler Jan. 14, 2021, 12:10 p.m. UTC | #1 
> -----Original Message-----
> From: openwrt-devel [mailto:openwrt-devel-bounces@lists.openwrt.org]
> On Behalf Of Pavel Šimerda
> Sent: Donnerstag, 14. Januar 2021 03:39
> To: openwrt-devel@lists.openwrt.org
> Cc: Pavel Šimerda <code@simerda.eu>
> Subject: [PATCH 1/2] libnl: add libnl-cli library

Hi,

this has formal issues (like entirely missing commit message, missing SoB, ...). Please have another look at https://openwrt.org/submitting-patches

Apart from that, do we need this in the main repo or could it also go into packages?

Best

Adrian

> 
> ---
>  package/libs/libnl/Makefile | 19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/package/libs/libnl/Makefile b/package/libs/libnl/Makefile index
> db0c65c7a7..3b9bad4533 100644
> --- a/package/libs/libnl/Makefile
> +++ b/package/libs/libnl/Makefile
> @@ -52,16 +52,26 @@ $(call Package/libnl/default)
>    DEPENDS:=+libnl-route
>  endef
> 
> +define Package/libnl-cli
> +$(call Package/libnl/default)
> +  TITLE:=Netlink Library CLI
> +  DEPENDS:=+libnl-genl +libnl-route +libnl-nf endef
> +
>  define Package/libnl
>  $(call Package/libnl/default)
>    TITLE:=Full Netlink Library
> -  DEPENDS:=+libnl-genl +libnl-route +libnl-nf
> +  DEPENDS:=+libnl-genl +libnl-route +libnl-nf +libnl-cli
>  endef
> 
>  define Package/libnl-core/description
>   Common code for all netlink libraries
>  endef
> 
> +define Package/libnl-cli/description
> + CLI Netlink Library Functions
> +endef
> +
>  define Package/libnl-genl/description
>   Generic Netlink Library Functions
>  endef
> @@ -92,6 +102,7 @@ define Build/InstallDev
> 
>  	# Copy symlinks
>  	$(CP) $(PKG_INSTALL_DIR)/usr/lib/libnl-3.so $(1)/usr/lib/libnl.so
> +	$(CP) $(PKG_INSTALL_DIR)/usr/lib/libnl-cli-3.so $(1)/usr/lib/libnl.so
>  	$(CP) $(PKG_INSTALL_DIR)/usr/lib/libnl-genl-3.so $(1)/usr/lib/libnl-
> genl.so
>  	$(CP) $(PKG_INSTALL_DIR)/usr/lib/libnl-nf-3.so $(1)/usr/lib/libnl-
> nf.so
>  	$(CP) $(PKG_INSTALL_DIR)/usr/lib/libnl-route-3.so $(1)/usr/lib/libnl-
> route.so @@ -102,6 +113,11 @@ define Package/libnl-core/install
>  	$(CP) $(PKG_INSTALL_DIR)/usr/lib/libnl-3.so.* $(1)/usr/lib/  endef
> 
> +define Package/libnl-cli/install
> +	$(INSTALL_DIR) $(1)/usr/lib
> +	$(CP) $(PKG_INSTALL_DIR)/usr/lib/libnl-cli-3.so.* $(1)/usr/lib/ endef
> +
>  define Package/libnl-genl/install
>  	$(INSTALL_DIR) $(1)/usr/lib
>  	$(CP) $(PKG_INSTALL_DIR)/usr/lib/libnl-genl-3.so.* $(1)/usr/lib/ @@
> -122,6 +138,7 @@ define Package/libnl/install  endef
> 
>  $(eval $(call BuildPackage,libnl-core))
> +$(eval $(call BuildPackage,libnl-cli))
>  $(eval $(call BuildPackage,libnl-genl))  $(eval $(call BuildPackage,libnl-route))
> $(eval $(call BuildPackage,libnl-nf))
> --
> 2.29.2
> 
> 
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Pavel Šimerda Jan. 15, 2021, 11:23 p.m. UTC | #2 
On 1/14/21 1:10 PM, Adrian Schmutzler wrote:
>> -----Original Message-----
>> From: openwrt-devel [mailto:openwrt-devel-bounces@lists.openwrt.org]
>> On Behalf Of Pavel Šimerda
>> Sent: Donnerstag, 14. Januar 2021 03:39
>> To: openwrt-devel@lists.openwrt.org
>> Cc: Pavel Šimerda <code@simerda.eu>
>> Subject: [PATCH 1/2] libnl: add libnl-cli library
> 
> Hi,
> 
> this has formal issues (like entirely missing commit message, missing SoB, ...). Please have another look at https://openwrt.org/submitting-patches
Hey,


thanks for your quick response to my first submission. I'll fix that.

> Apart from that, do we need this in the main repo or could it also go into packages?

Whether libteam belongs to the main repo or package may be subject to point of view.



My perspective is that we are working on *hardware enablement* here. At least some switch chips support Link Aggregation in addition to hardware switching and VLAN filtering. This will be handled by offloading the LAG configuration from the kernel team driver.



The intended functionality consists of:



   * Team module for netifd
  (package already in main)
       - Submitted: https://lists.openwrt.org/pipermail/openwrt-devel/2021-January/033269.html

   * The libteam package to support the team module (new package)
       - Submitted in PATCH 2/2.

   * A change in libnl to support the libteam package (package already in main)
       - Submitted in PATCH 1/2.

   * The kernel team driver (already in kernel)
       - Works as expected.

   * Offloaded LAG confugration to a switch driver (to be done in kernel)
       - This is of course hardware specific.

I would rather see all support protocols for switch chip features (like RSTP and LACP) ready in the main repo and supported by netifd and the network UCI configuration. What is the motivation to keep these networking features separate?

Cheers

Pavel Šimerda

> Best
> 
> Adrian
> 
>>
>> ---
>>   package/libs/libnl/Makefile | 19 ++++++++++++++++++-
>>   1 file changed, 18 insertions(+), 1 deletion(-)
>>
>> diff --git a/package/libs/libnl/Makefile b/package/libs/libnl/Makefile index
>> db0c65c7a7..3b9bad4533 100644
>> --- a/package/libs/libnl/Makefile
>> +++ b/package/libs/libnl/Makefile
>> @@ -52,16 +52,26 @@ $(call Package/libnl/default)
>>     DEPENDS:=+libnl-route
>>   endef
>>
>> +define Package/libnl-cli
>> +$(call Package/libnl/default)
>> +  TITLE:=Netlink Library CLI
>> +  DEPENDS:=+libnl-genl +libnl-route +libnl-nf endef
>> +
>>   define Package/libnl
>>   $(call Package/libnl/default)
>>     TITLE:=Full Netlink Library
>> -  DEPENDS:=+libnl-genl +libnl-route +libnl-nf
>> +  DEPENDS:=+libnl-genl +libnl-route +libnl-nf +libnl-cli
>>   endef
>>
>>   define Package/libnl-core/description
>>    Common code for all netlink libraries
>>   endef
>>
>> +define Package/libnl-cli/description
>> + CLI Netlink Library Functions
>> +endef
>> +
>>   define Package/libnl-genl/description
>>    Generic Netlink Library Functions
>>   endef
>> @@ -92,6 +102,7 @@ define Build/InstallDev
>>
>>   	# Copy symlinks
>>   	$(CP) $(PKG_INSTALL_DIR)/usr/lib/libnl-3.so $(1)/usr/lib/libnl.so
>> +	$(CP) $(PKG_INSTALL_DIR)/usr/lib/libnl-cli-3.so $(1)/usr/lib/libnl.so
>>   	$(CP) $(PKG_INSTALL_DIR)/usr/lib/libnl-genl-3.so $(1)/usr/lib/libnl-
>> genl.so
>>   	$(CP) $(PKG_INSTALL_DIR)/usr/lib/libnl-nf-3.so $(1)/usr/lib/libnl-
>> nf.so
>>   	$(CP) $(PKG_INSTALL_DIR)/usr/lib/libnl-route-3.so $(1)/usr/lib/libnl-
>> route.so @@ -102,6 +113,11 @@ define Package/libnl-core/install
>>   	$(CP) $(PKG_INSTALL_DIR)/usr/lib/libnl-3.so.* $(1)/usr/lib/  endef
>>
>> +define Package/libnl-cli/install
>> +	$(INSTALL_DIR) $(1)/usr/lib
>> +	$(CP) $(PKG_INSTALL_DIR)/usr/lib/libnl-cli-3.so.* $(1)/usr/lib/ endef
>> +
>>   define Package/libnl-genl/install
>>   	$(INSTALL_DIR) $(1)/usr/lib
>>   	$(CP) $(PKG_INSTALL_DIR)/usr/lib/libnl-genl-3.so.* $(1)/usr/lib/ @@
>> -122,6 +138,7 @@ define Package/libnl/install  endef
>>
>>   $(eval $(call BuildPackage,libnl-core))
>> +$(eval $(call BuildPackage,libnl-cli))
>>   $(eval $(call BuildPackage,libnl-genl))  $(eval $(call BuildPackage,libnl-route))
>> $(eval $(call BuildPackage,libnl-nf))
>> --
>> 2.29.2
>>
>>
>> _______________________________________________
>> openwrt-devel mailing list
>> openwrt-devel@lists.openwrt.org
>> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
>>
>> _______________________________________________
>> openwrt-devel mailing list
>> openwrt-devel@lists.openwrt.org
>> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Petr Štetiar Jan. 16, 2021, 11:25 a.m. UTC | #3 
Pavel Šimerda <code@simerda.eu> [2021-01-14 03:38:47]:

Hi,

> ---
>  package/libs/libnl/Makefile | 19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/package/libs/libnl/Makefile b/package/libs/libnl/Makefile
> index db0c65c7a7..3b9bad4533 100644
> --- a/package/libs/libnl/Makefile
> +++ b/package/libs/libnl/Makefile
> @@ -52,16 +52,26 @@ $(call Package/libnl/default)
>    DEPENDS:=+libnl-route
>  endef
>  
> +define Package/libnl-cli
> +$(call Package/libnl/default)
> +  TITLE:=Netlink Library CLI
> +  DEPENDS:=+libnl-genl +libnl-route +libnl-nf
> +endef
> +
>  define Package/libnl
>  $(call Package/libnl/default)
>    TITLE:=Full Netlink Library
> -  DEPENDS:=+libnl-genl +libnl-route +libnl-nf
> +  DEPENDS:=+libnl-genl +libnl-route +libnl-nf +libnl-cli

Why is this dependency needed?

>  endef
>  
>  define Package/libnl-core/description
>   Common code for all netlink libraries
>  endef
>  
> +define Package/libnl-cli/description
> + CLI Netlink Library Functions
> +endef
> +
>  define Package/libnl-genl/description
>   Generic Netlink Library Functions
>  endef
> @@ -92,6 +102,7 @@ define Build/InstallDev
>  
>  	# Copy symlinks
>  	$(CP) $(PKG_INSTALL_DIR)/usr/lib/libnl-3.so $(1)/usr/lib/libnl.so
> +	$(CP) $(PKG_INSTALL_DIR)/usr/lib/libnl-cli-3.so $(1)/usr/lib/libnl.so

This seems like copy&pasta issue.

>  	$(CP) $(PKG_INSTALL_DIR)/usr/lib/libnl-genl-3.so $(1)/usr/lib/libnl-genl.so
>  	$(CP) $(PKG_INSTALL_DIR)/usr/lib/libnl-nf-3.so $(1)/usr/lib/libnl-nf.so
>  	$(CP) $(PKG_INSTALL_DIR)/usr/lib/libnl-route-3.so $(1)/usr/lib/libnl-route.so
> @@ -102,6 +113,11 @@ define Package/libnl-core/install
>  	$(CP) $(PKG_INSTALL_DIR)/usr/lib/libnl-3.so.* $(1)/usr/lib/
>  endef
>  
> +define Package/libnl-cli/install
> +	$(INSTALL_DIR) $(1)/usr/lib
> +	$(CP) $(PKG_INSTALL_DIR)/usr/lib/libnl-cli-3.so.* $(1)/usr/lib/
> +endef
> +
>  define Package/libnl-genl/install
>  	$(INSTALL_DIR) $(1)/usr/lib
>  	$(CP) $(PKG_INSTALL_DIR)/usr/lib/libnl-genl-3.so.* $(1)/usr/lib/
> @@ -122,6 +138,7 @@ define Package/libnl/install
>  endef
>  
>  $(eval $(call BuildPackage,libnl-core))
> +$(eval $(call BuildPackage,libnl-cli))
>  $(eval $(call BuildPackage,libnl-genl))
>  $(eval $(call BuildPackage,libnl-route))
>  $(eval $(call BuildPackage,libnl-nf))
> -- 
> 2.29.2
> 
> 
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
>
Petr Štetiar Jan. 16, 2021, 11:26 a.m. UTC | #4 
Adrian Schmutzler <mail@adrianschmutzler.de> [2021-01-14 13:10:14]:

Hi,

> Apart from that, do we need this in the main repo or could it also go into packages?

in this case no.

-- ynezz
Petr Štetiar Jan. 16, 2021, 11:50 a.m. UTC | #5 
Pavel Šimerda <code@simerda.eu> [2021-01-16 00:23:01]:

Hi,

> My perspective is that we are working on *hardware enablement* here. At
> least some switch chips support Link Aggregation in addition to hardware
> switching and VLAN filtering. This will be handled by offloading the LAG
> configuration from the kernel team driver.

we're trying to keep the main tree minimal for details see following link:

 https://lists.infradead.org/pipermail/openwrt-devel/2019-August/024109.html

>   * The libteam package to support the team module (new package)
>       - Submitted in PATCH 2/2.

This one looks like material for packages feed.

-- ynezz
Pavel Šimerda Jan. 16, 2021, 12:04 p.m. UTC | #6 
On 1/16/21 12:25 PM, Petr Štetiar wrote:
> Pavel Šimerda <code@simerda.eu> [2021-01-14 03:38:47]:
> 
> Hi,
> 
>> ---
>>   package/libs/libnl/Makefile | 19 ++++++++++++++++++-
>>   1 file changed, 18 insertions(+), 1 deletion(-)
>>
>> diff --git a/package/libs/libnl/Makefile b/package/libs/libnl/Makefile
>> index db0c65c7a7..3b9bad4533 100644
>> --- a/package/libs/libnl/Makefile
>> +++ b/package/libs/libnl/Makefile
>> @@ -52,16 +52,26 @@ $(call Package/libnl/default)
>>     DEPENDS:=+libnl-route
>>   endef
>>   
>> +define Package/libnl-cli
>> +$(call Package/libnl/default)
>> +  TITLE:=Netlink Library CLI
>> +  DEPENDS:=+libnl-genl +libnl-route +libnl-nf
>> +endef
>> +
>>   define Package/libnl
>>   $(call Package/libnl/default)
>>     TITLE:=Full Netlink Library
>> -  DEPENDS:=+libnl-genl +libnl-route +libnl-nf
>> +  DEPENDS:=+libnl-genl +libnl-route +libnl-nf +libnl-cli
> 
> Why is this dependency needed?

Hey,

would you suggest that libnl-cli stays left out of libnl deps as an “extra” package? I have no strong opinion about that one.

> 
>>   endef
>>   
>>   define Package/libnl-core/description
>>    Common code for all netlink libraries
>>   endef
>>   
>> +define Package/libnl-cli/description
>> + CLI Netlink Library Functions
>> +endef
>> +
>>   define Package/libnl-genl/description
>>    Generic Netlink Library Functions
>>   endef
>> @@ -92,6 +102,7 @@ define Build/InstallDev
>>   
>>   	# Copy symlinks
>>   	$(CP) $(PKG_INSTALL_DIR)/usr/lib/libnl-3.so $(1)/usr/lib/libnl.so
>> +	$(CP) $(PKG_INSTALL_DIR)/usr/lib/libnl-cli-3.so $(1)/usr/lib/libnl.so
> 
> This seems like copy&pasta issue.

Thanks.

Pavel Šimerda

>>   	$(CP) $(PKG_INSTALL_DIR)/usr/lib/libnl-genl-3.so $(1)/usr/lib/libnl-genl.so
>>   	$(CP) $(PKG_INSTALL_DIR)/usr/lib/libnl-nf-3.so $(1)/usr/lib/libnl-nf.so
>>   	$(CP) $(PKG_INSTALL_DIR)/usr/lib/libnl-route-3.so $(1)/usr/lib/libnl-route.so
>> @@ -102,6 +113,11 @@ define Package/libnl-core/install
>>   	$(CP) $(PKG_INSTALL_DIR)/usr/lib/libnl-3.so.* $(1)/usr/lib/
>>   endef
>>   
>> +define Package/libnl-cli/install
>> +	$(INSTALL_DIR) $(1)/usr/lib
>> +	$(CP) $(PKG_INSTALL_DIR)/usr/lib/libnl-cli-3.so.* $(1)/usr/lib/
>> +endef
>> +
>>   define Package/libnl-genl/install
>>   	$(INSTALL_DIR) $(1)/usr/lib
>>   	$(CP) $(PKG_INSTALL_DIR)/usr/lib/libnl-genl-3.so.* $(1)/usr/lib/
>> @@ -122,6 +138,7 @@ define Package/libnl/install
>>   endef
>>   
>>   $(eval $(call BuildPackage,libnl-core))
>> +$(eval $(call BuildPackage,libnl-cli))
>>   $(eval $(call BuildPackage,libnl-genl))
>>   $(eval $(call BuildPackage,libnl-route))
>>   $(eval $(call BuildPackage,libnl-nf))
>> -- 
>> 2.29.2
>>
>>
>> _______________________________________________
>> openwrt-devel mailing list
>> openwrt-devel@lists.openwrt.org
>> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
>>
>
Pavel Šimerda Jan. 16, 2021, 12:18 p.m. UTC | #7 
On 1/16/21 12:50 PM, Petr Štetiar wrote:
> Pavel Šimerda <code@simerda.eu> [2021-01-16 00:23:01]:
> 
> Hi,
> 
>> My perspective is that we are working on *hardware enablement* here. At
>> least some switch chips support Link Aggregation in addition to hardware
>> switching and VLAN filtering. This will be handled by offloading the LAG
>> configuration from the kernel team driver.
> 
> we're trying to keep the main tree minimal for details see following link:
> 
>   https://lists.infradead.org/pipermail/openwrt-devel/2019-August/024109.html

Hey Petr,

is this an idea in progress or is OpenWRT already heading towards that goal?

> 
>>    * The libteam package to support the team module (new package)
>>        - Submitted in PATCH 2/2.
> 
> This one looks like material for packages feed.

So what is the correct approach? Please see my questions below.

1) Is it okay to modify libnl to build and distribute the libnl-cli package so that I can base the libteam package on it?

2) How should I handle the netifd team module dependency on libteam to get it accepted to netifd code base?

Cheers,

Pavel Šimerda

> 
> -- ynezz
>
Paul Spooren Jan. 17, 2021, 12:05 a.m. UTC | #8 
Jan 16, 2021 2:19:15 AM Pavel Šimerda <code@simerda.eu>:

> On 1/16/21 12:50 PM, Petr Štetiar wrote:
>> Pavel Šimerda <code@simerda.eu> [2021-01-16 00:23:01]:
>> Hi,
>> My perspective is that we are working on *hardware enablement* here. 
At
>>> least some switch chips support Link Aggregation in addition to 
hardware
>>> switching and VLAN filtering. This will be handled by offloading the 
LAG
>>> configuration from the kernel team driver.
>> we're trying to keep the main tree minimal for details see following 
link:
>>   
https://lists.infradead.org/pipermail/openwrt-devel/2019-August/024109.html
>
> Hey Petr,
>
> is this an idea in progress or is OpenWRT already heading towards that 
goal?

It's an ongoing progress. If you look at the Git log multiple packages 
were moved over the last weeks.

>
>
>>    * The libteam package to support the team module (new package)
>>>        - Submitted in PATCH 2/2.
>> This one looks like material for packages feed.
>
> So what is the correct approach? Please see my questions below.
>
> 1) Is it okay to modify libnl to build and distribute the libnl-cli 
package so that I can base the libteam package on it?
>
> 2) How should I handle the netifd team module dependency on libteam to 
get it accepted to netifd code base?
>
> Cheers,
>
> Pavel Šimerda
>
>> -- ynezz
>>
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
diff mbox series

Patch

diff --git a/package/libs/libnl/Makefile b/package/libs/libnl/Makefile
index db0c65c7a7..3b9bad4533 100644
--- a/package/libs/libnl/Makefile
+++ b/package/libs/libnl/Makefile
@@ -52,16 +52,26 @@  $(call Package/libnl/default)
   DEPENDS:=+libnl-route
 endef
 
+define Package/libnl-cli
+$(call Package/libnl/default)
+  TITLE:=Netlink Library CLI
+  DEPENDS:=+libnl-genl +libnl-route +libnl-nf
+endef
+
 define Package/libnl
 $(call Package/libnl/default)
   TITLE:=Full Netlink Library
-  DEPENDS:=+libnl-genl +libnl-route +libnl-nf
+  DEPENDS:=+libnl-genl +libnl-route +libnl-nf +libnl-cli
 endef
 
 define Package/libnl-core/description
  Common code for all netlink libraries
 endef
 
+define Package/libnl-cli/description
+ CLI Netlink Library Functions
+endef
+
 define Package/libnl-genl/description
  Generic Netlink Library Functions
 endef
@@ -92,6 +102,7 @@  define Build/InstallDev
 
 	# Copy symlinks
 	$(CP) $(PKG_INSTALL_DIR)/usr/lib/libnl-3.so $(1)/usr/lib/libnl.so
+	$(CP) $(PKG_INSTALL_DIR)/usr/lib/libnl-cli-3.so $(1)/usr/lib/libnl.so
 	$(CP) $(PKG_INSTALL_DIR)/usr/lib/libnl-genl-3.so $(1)/usr/lib/libnl-genl.so
 	$(CP) $(PKG_INSTALL_DIR)/usr/lib/libnl-nf-3.so $(1)/usr/lib/libnl-nf.so
 	$(CP) $(PKG_INSTALL_DIR)/usr/lib/libnl-route-3.so $(1)/usr/lib/libnl-route.so
@@ -102,6 +113,11 @@  define Package/libnl-core/install
 	$(CP) $(PKG_INSTALL_DIR)/usr/lib/libnl-3.so.* $(1)/usr/lib/
 endef
 
+define Package/libnl-cli/install
+	$(INSTALL_DIR) $(1)/usr/lib
+	$(CP) $(PKG_INSTALL_DIR)/usr/lib/libnl-cli-3.so.* $(1)/usr/lib/
+endef
+
 define Package/libnl-genl/install
 	$(INSTALL_DIR) $(1)/usr/lib
 	$(CP) $(PKG_INSTALL_DIR)/usr/lib/libnl-genl-3.so.* $(1)/usr/lib/
@@ -122,6 +138,7 @@  define Package/libnl/install
 endef
 
 $(eval $(call BuildPackage,libnl-core))
+$(eval $(call BuildPackage,libnl-cli))
 $(eval $(call BuildPackage,libnl-genl))
 $(eval $(call BuildPackage,libnl-route))
 $(eval $(call BuildPackage,libnl-nf))