* Re: [PATCH 07/17] netconfig: Use an internal rtnl socket for l_netconfig_apply_rtnl
@ 2022-05-19 11:52 Andrew Zaborowski
0 siblings, 0 replies; 8+ messages in thread
From: Andrew Zaborowski @ 2022-05-19 11:52 UTC (permalink / raw)
To: ell
[-- Attachment #1: Type: text/plain, Size: 957 bytes --]
Hi Denis,
On Wed, 18 May 2022 at 20:58, Denis Kenzior <denkenz(a)gmail.com> wrote:
> On 5/13/22 17:47, Andrew Zaborowski wrote:
> > LIB_EXPORT struct l_netconfig *l_netconfig_new(uint32_t ifindex)
> > {
> > struct l_netconfig *nc;
> > + struct l_netlink *rtnl = l_netlink_new(NETLINK_ROUTE);
> > +
> > + if (!rtnl)
> > + return NULL;
>
> I'm against this. We will have multiple netconfig objects in play and each one
> would be utilizing an fd. We should be keeping our fd use to a minimum,
> particularly since we're limited to ~128 right now. Passing in the rtnl object
> from outside is preferred.
Makes sense. How about using a global rtnl object for all of the
l_netconfig objects? Do you still prefer that it be passed from
outside?
I did this later in patch 11 and maybe should have merged that change
directly here. We could also create it only after
l_netconfig_start().
Best regards
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 07/17] netconfig: Use an internal rtnl socket for l_netconfig_apply_rtnl
@ 2022-05-19 21:38 Andrew Zaborowski
0 siblings, 0 replies; 8+ messages in thread
From: Andrew Zaborowski @ 2022-05-19 21:38 UTC (permalink / raw)
To: ell
[-- Attachment #1: Type: text/plain, Size: 1734 bytes --]
On Thu, 19 May 2022 at 23:28, Denis Kenzior <denkenz(a)gmail.com> wrote:
> On 5/19/22 15:53, Andrew Zaborowski wrote:
> > On Thu, 19 May 2022 at 22:28, Denis Kenzior <denkenz(a)gmail.com> wrote:
> >>> Makes sense. How about using a global rtnl object for all of the
> >>> l_netconfig objects? Do you still prefer that it be passed from
> >>> outside?
> >>
> >> I have no real preference on whether it is passed in from the outside vs not.
> >> But, there's another thing to consider here. Today the call to l_foo_set_rtnl
> >> is used determine whether the dhcp/icmp/dhcp6 object should set everything up
> >> directly vs letting the outside layer do this. With l_netconfig I don't think
> >> this is a problem per se, but you should consider the other classes when you
> >> design this.
> >
> > Right but with l_netconfig you make this choice by either calling
> > l_netconfig_apply_rtnl or not, and you shouldn't call l_foo_set_rtnl
> > on the underlying dhcp/icmp objects.
> >
>
> Sure, but are you assuming we will not have any further classes using rtnl
> besides netconfig?
>
> >>
> >> We can start by trying to come up with a nice ell API for obtaining the
> >> singleton rtnl object and go from there.
> >
> > The global rtnl object I meant is to be used only by l_netconfig
> > instances so there's no need for a getter, or I'm misunderstanding
> > what you're saying.
> >
>
> We use rtnl all over the place in iwd, like netdev, p2p, ap, etc. All these +
> netconfig should share the same one.
That's different than what I did in patch 11/17 but it would work.
I'll put it in rtnl.c then, with a _get() and a _put() since we have
no generic refcounting for l_netlink.
Best refards
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 07/17] netconfig: Use an internal rtnl socket for l_netconfig_apply_rtnl
@ 2022-05-19 21:28 Denis Kenzior
0 siblings, 0 replies; 8+ messages in thread
From: Denis Kenzior @ 2022-05-19 21:28 UTC (permalink / raw)
To: ell
[-- Attachment #1: Type: text/plain, Size: 1444 bytes --]
Hi Andrew,
On 5/19/22 15:53, Andrew Zaborowski wrote:
> On Thu, 19 May 2022 at 22:28, Denis Kenzior <denkenz(a)gmail.com> wrote:
>>> Makes sense. How about using a global rtnl object for all of the
>>> l_netconfig objects? Do you still prefer that it be passed from
>>> outside?
>>
>> I have no real preference on whether it is passed in from the outside vs not.
>> But, there's another thing to consider here. Today the call to l_foo_set_rtnl
>> is used determine whether the dhcp/icmp/dhcp6 object should set everything up
>> directly vs letting the outside layer do this. With l_netconfig I don't think
>> this is a problem per se, but you should consider the other classes when you
>> design this.
>
> Right but with l_netconfig you make this choice by either calling
> l_netconfig_apply_rtnl or not, and you shouldn't call l_foo_set_rtnl
> on the underlying dhcp/icmp objects.
>
Sure, but are you assuming we will not have any further classes using rtnl
besides netconfig?
>>
>> We can start by trying to come up with a nice ell API for obtaining the
>> singleton rtnl object and go from there.
>
> The global rtnl object I meant is to be used only by l_netconfig
> instances so there's no need for a getter, or I'm misunderstanding
> what you're saying.
>
We use rtnl all over the place in iwd, like netdev, p2p, ap, etc. All these +
netconfig should share the same one.
Regards,
-Denis
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 07/17] netconfig: Use an internal rtnl socket for l_netconfig_apply_rtnl
@ 2022-05-19 20:53 Andrew Zaborowski
0 siblings, 0 replies; 8+ messages in thread
From: Andrew Zaborowski @ 2022-05-19 20:53 UTC (permalink / raw)
To: ell
[-- Attachment #1: Type: text/plain, Size: 1126 bytes --]
On Thu, 19 May 2022 at 22:28, Denis Kenzior <denkenz(a)gmail.com> wrote:
> > Makes sense. How about using a global rtnl object for all of the
> > l_netconfig objects? Do you still prefer that it be passed from
> > outside?
>
> I have no real preference on whether it is passed in from the outside vs not.
> But, there's another thing to consider here. Today the call to l_foo_set_rtnl
> is used determine whether the dhcp/icmp/dhcp6 object should set everything up
> directly vs letting the outside layer do this. With l_netconfig I don't think
> this is a problem per se, but you should consider the other classes when you
> design this.
Right but with l_netconfig you make this choice by either calling
l_netconfig_apply_rtnl or not, and you shouldn't call l_foo_set_rtnl
on the underlying dhcp/icmp objects.
>
> We can start by trying to come up with a nice ell API for obtaining the
> singleton rtnl object and go from there.
The global rtnl object I meant is to be used only by l_netconfig
instances so there's no need for a getter, or I'm misunderstanding
what you're saying.
Best regards
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 07/17] netconfig: Use an internal rtnl socket for l_netconfig_apply_rtnl
@ 2022-05-19 20:28 Denis Kenzior
0 siblings, 0 replies; 8+ messages in thread
From: Denis Kenzior @ 2022-05-19 20:28 UTC (permalink / raw)
To: ell
[-- Attachment #1: Type: text/plain, Size: 716 bytes --]
Hi Andrew,
> Makes sense. How about using a global rtnl object for all of the
> l_netconfig objects? Do you still prefer that it be passed from
> outside?
I have no real preference on whether it is passed in from the outside vs not.
But, there's another thing to consider here. Today the call to l_foo_set_rtnl
is used determine whether the dhcp/icmp/dhcp6 object should set everything up
directly vs letting the outside layer do this. With l_netconfig I don't think
this is a problem per se, but you should consider the other classes when you
design this.
We can start by trying to come up with a nice ell API for obtaining the
singleton rtnl object and go from there.
Regards,
-Denis
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 07/17] netconfig: Use an internal rtnl socket for l_netconfig_apply_rtnl
@ 2022-05-18 18:58 Denis Kenzior
0 siblings, 0 replies; 8+ messages in thread
From: Denis Kenzior @ 2022-05-18 18:58 UTC (permalink / raw)
To: ell
[-- Attachment #1: Type: text/plain, Size: 986 bytes --]
Hi Andrew,
On 5/13/22 17:47, Andrew Zaborowski wrote:
> Since we'll need to keep an internal RTNL socket for DHCPv6
> functionality anyway, change l_netconfig_apply_rtnl's signature to not
> require an rtnl parameter.
> ---
> ell/netconfig.c | 34 ++++++++++++++++++++--------------
> ell/netconfig.h | 3 +--
> 2 files changed, 21 insertions(+), 16 deletions(-)
>
<snip>
> @@ -730,8 +731,13 @@ static void netconfig_icmp6_event_handler(struct l_icmp6_client *client,
> LIB_EXPORT struct l_netconfig *l_netconfig_new(uint32_t ifindex)
> {
> struct l_netconfig *nc;
> + struct l_netlink *rtnl = l_netlink_new(NETLINK_ROUTE);
> +
> + if (!rtnl)
> + return NULL;
I'm against this. We will have multiple netconfig objects in play and each one
would be utilizing an fd. We should be keeping our fd use to a minimum,
particularly since we're limited to ~128 right now. Passing in the rtnl object
from outside is preferred.
Regards,
-Denis
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 07/17] netconfig: Use an internal rtnl socket for l_netconfig_apply_rtnl
@ 2022-05-13 22:47 Andrew Zaborowski
0 siblings, 0 replies; 8+ messages in thread
From: Andrew Zaborowski @ 2022-05-13 22:47 UTC (permalink / raw)
To: ell
[-- Attachment #1: Type: text/plain, Size: 4246 bytes --]
Since we'll need to keep an internal RTNL socket for DHCPv6
functionality anyway, change l_netconfig_apply_rtnl's signature to not
require an rtnl parameter.
---
ell/netconfig.c | 34 ++++++++++++++++++++--------------
ell/netconfig.h | 3 +--
2 files changed, 21 insertions(+), 16 deletions(-)
diff --git a/ell/netconfig.c b/ell/netconfig.c
index 94d3ed1..55a7993 100644
--- a/ell/netconfig.c
+++ b/ell/netconfig.c
@@ -64,6 +64,7 @@ struct l_netconfig {
char **v6_dns_override;
char **v6_domain_names_override;
+ struct l_netlink *rtnl;
bool started;
struct l_idle *do_static_work;
bool v4_configured;
@@ -730,8 +731,13 @@ static void netconfig_icmp6_event_handler(struct l_icmp6_client *client,
LIB_EXPORT struct l_netconfig *l_netconfig_new(uint32_t ifindex)
{
struct l_netconfig *nc;
+ struct l_netlink *rtnl = l_netlink_new(NETLINK_ROUTE);
+
+ if (!rtnl)
+ return NULL;
nc = l_new(struct l_netconfig, 1);
+ nc->rtnl = rtnl;
nc->ifindex = ifindex;
nc->v4_enabled = true;
@@ -789,6 +795,7 @@ LIB_EXPORT void l_netconfig_destroy(struct l_netconfig *netconfig)
l_queue_destroy(netconfig->routes.added, NULL);
l_queue_destroy(netconfig->routes.updated, NULL);
l_queue_destroy(netconfig->routes.removed, NULL);
+ l_netlink_destroy(netconfig->rtnl);
l_free(netconfig);
}
@@ -1193,42 +1200,41 @@ LIB_EXPORT void l_netconfig_set_event_handler(struct l_netconfig *netconfig,
netconfig->handler.destroy = destroy;
}
-LIB_EXPORT void l_netconfig_apply_rtnl(struct l_netconfig *netconfig,
- struct l_netlink *rtnl)
+LIB_EXPORT void l_netconfig_apply_rtnl(struct l_netconfig *netconfig)
{
const struct l_queue_entry *entry;
for (entry = l_queue_get_entries(netconfig->addresses.removed); entry;
entry = entry->next)
- l_rtnl_ifaddr_delete(rtnl, netconfig->ifindex, entry->data,
- NULL, NULL, NULL);
+ l_rtnl_ifaddr_delete(netconfig->rtnl, netconfig->ifindex,
+ entry->data, NULL, NULL, NULL);
for (entry = l_queue_get_entries(netconfig->addresses.added); entry;
entry = entry->next)
- l_rtnl_ifaddr_add(rtnl, netconfig->ifindex, entry->data,
- NULL, NULL, NULL);
+ l_rtnl_ifaddr_add(netconfig->rtnl, netconfig->ifindex,
+ entry->data, NULL, NULL, NULL);
/* We can use l_rtnl_ifaddr_add here since that uses NLM_F_REPLACE */
for (entry = l_queue_get_entries(netconfig->addresses.updated); entry;
entry = entry->next)
- l_rtnl_ifaddr_add(rtnl, netconfig->ifindex, entry->data,
- NULL, NULL, NULL);
+ l_rtnl_ifaddr_add(netconfig->rtnl, netconfig->ifindex,
+ entry->data, NULL, NULL, NULL);
for (entry = l_queue_get_entries(netconfig->routes.removed); entry;
entry = entry->next)
- l_rtnl_route_delete(rtnl, netconfig->ifindex, entry->data,
- NULL, NULL, NULL);
+ l_rtnl_route_delete(netconfig->rtnl, netconfig->ifindex,
+ entry->data, NULL, NULL, NULL);
for (entry = l_queue_get_entries(netconfig->routes.added); entry;
entry = entry->next)
- l_rtnl_route_add(rtnl, netconfig->ifindex, entry->data,
- NULL, NULL, NULL);
+ l_rtnl_route_add(netconfig->rtnl, netconfig->ifindex,
+ entry->data, NULL, NULL, NULL);
/* We can use l_rtnl_route_add here since that uses NLM_F_REPLACE */
for (entry = l_queue_get_entries(netconfig->routes.updated); entry;
entry = entry->next)
- l_rtnl_route_add(rtnl, netconfig->ifindex, entry->data,
- NULL, NULL, NULL);
+ l_rtnl_route_add(netconfig->rtnl, netconfig->ifindex,
+ entry->data, NULL, NULL, NULL);
}
LIB_EXPORT const struct l_queue_entry *l_netconfig_get_addresses(
diff --git a/ell/netconfig.h b/ell/netconfig.h
index d58dda1..874491f 100644
--- a/ell/netconfig.h
+++ b/ell/netconfig.h
@@ -81,8 +81,7 @@ void l_netconfig_set_event_handler(struct l_netconfig *netconfig,
void *user_data,
l_netconfig_destroy_cb_t destroy);
-void l_netconfig_apply_rtnl(struct l_netconfig *netconfig,
- struct l_netlink *rtnl);
+void l_netconfig_apply_rtnl(struct l_netconfig *netconfig);
const struct l_queue_entry *l_netconfig_get_addresses(
struct l_netconfig *netconfig,
const struct l_queue_entry **out_added,
--
2.32.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 07/17] netconfig: Use an internal rtnl socket for l_netconfig_apply_rtnl
@ 2022-05-13 14:55 Andrew Zaborowski
0 siblings, 0 replies; 8+ messages in thread
From: Andrew Zaborowski @ 2022-05-13 14:55 UTC (permalink / raw)
To: ell
[-- Attachment #1: Type: text/plain, Size: 4246 bytes --]
Since we'll need to keep an internal RTNL socket for DHCPv6
functionality anyway, change l_netconfig_apply_rtnl's signature to not
require an rtnl parameter.
---
ell/netconfig.c | 34 ++++++++++++++++++++--------------
ell/netconfig.h | 3 +--
2 files changed, 21 insertions(+), 16 deletions(-)
diff --git a/ell/netconfig.c b/ell/netconfig.c
index aee56a2..724cb38 100644
--- a/ell/netconfig.c
+++ b/ell/netconfig.c
@@ -63,6 +63,7 @@ struct l_netconfig {
char **v6_dns_override;
char **v6_domain_names_override;
+ struct l_netlink *rtnl;
bool started;
struct l_idle *do_static_work;
bool v4_configured;
@@ -725,8 +726,13 @@ static void netconfig_icmp6_event_handler(struct l_icmp6_client *client,
LIB_EXPORT struct l_netconfig *l_netconfig_new(uint32_t ifindex)
{
struct l_netconfig *nc;
+ struct l_netlink *rtnl = l_netlink_new(NETLINK_ROUTE);
+
+ if (!rtnl)
+ return NULL;
nc = l_new(struct l_netconfig, 1);
+ nc->rtnl = rtnl;
nc->ifindex = ifindex;
nc->v4_enabled = true;
@@ -784,6 +790,7 @@ LIB_EXPORT void l_netconfig_destroy(struct l_netconfig *netconfig)
l_queue_destroy(netconfig->routes.added, NULL);
l_queue_destroy(netconfig->routes.updated, NULL);
l_queue_destroy(netconfig->routes.removed, NULL);
+ l_netlink_destroy(netconfig->rtnl);
l_free(netconfig);
}
@@ -1188,42 +1195,41 @@ LIB_EXPORT void l_netconfig_set_event_handler(struct l_netconfig *netconfig,
netconfig->handler.destroy = destroy;
}
-LIB_EXPORT void l_netconfig_apply_rtnl(struct l_netconfig *netconfig,
- struct l_netlink *rtnl)
+LIB_EXPORT void l_netconfig_apply_rtnl(struct l_netconfig *netconfig)
{
const struct l_queue_entry *entry;
for (entry = l_queue_get_entries(netconfig->addresses.removed); entry;
entry = entry->next)
- l_rtnl_ifaddr_delete(rtnl, netconfig->ifindex, entry->data,
- NULL, NULL, NULL);
+ l_rtnl_ifaddr_delete(netconfig->rtnl, netconfig->ifindex,
+ entry->data, NULL, NULL, NULL);
for (entry = l_queue_get_entries(netconfig->addresses.added); entry;
entry = entry->next)
- l_rtnl_ifaddr_add(rtnl, netconfig->ifindex, entry->data,
- NULL, NULL, NULL);
+ l_rtnl_ifaddr_add(netconfig->rtnl, netconfig->ifindex,
+ entry->data, NULL, NULL, NULL);
/* We can use l_rtnl_ifaddr_add here since that uses NLM_F_REPLACE */
for (entry = l_queue_get_entries(netconfig->addresses.updated); entry;
entry = entry->next)
- l_rtnl_ifaddr_add(rtnl, netconfig->ifindex, entry->data,
- NULL, NULL, NULL);
+ l_rtnl_ifaddr_add(netconfig->rtnl, netconfig->ifindex,
+ entry->data, NULL, NULL, NULL);
for (entry = l_queue_get_entries(netconfig->routes.removed); entry;
entry = entry->next)
- l_rtnl_route_delete(rtnl, netconfig->ifindex, entry->data,
- NULL, NULL, NULL);
+ l_rtnl_route_delete(netconfig->rtnl, netconfig->ifindex,
+ entry->data, NULL, NULL, NULL);
for (entry = l_queue_get_entries(netconfig->routes.added); entry;
entry = entry->next)
- l_rtnl_route_add(rtnl, netconfig->ifindex, entry->data,
- NULL, NULL, NULL);
+ l_rtnl_route_add(netconfig->rtnl, netconfig->ifindex,
+ entry->data, NULL, NULL, NULL);
/* We can use l_rtnl_route_add here since that uses NLM_F_REPLACE */
for (entry = l_queue_get_entries(netconfig->routes.updated); entry;
entry = entry->next)
- l_rtnl_route_add(rtnl, netconfig->ifindex, entry->data,
- NULL, NULL, NULL);
+ l_rtnl_route_add(netconfig->rtnl, netconfig->ifindex,
+ entry->data, NULL, NULL, NULL);
}
LIB_EXPORT const struct l_queue_entry *l_netconfig_get_addresses(
diff --git a/ell/netconfig.h b/ell/netconfig.h
index d58dda1..874491f 100644
--- a/ell/netconfig.h
+++ b/ell/netconfig.h
@@ -81,8 +81,7 @@ void l_netconfig_set_event_handler(struct l_netconfig *netconfig,
void *user_data,
l_netconfig_destroy_cb_t destroy);
-void l_netconfig_apply_rtnl(struct l_netconfig *netconfig,
- struct l_netlink *rtnl);
+void l_netconfig_apply_rtnl(struct l_netconfig *netconfig);
const struct l_queue_entry *l_netconfig_get_addresses(
struct l_netconfig *netconfig,
const struct l_queue_entry **out_added,
--
2.32.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-05-19 21:38 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-19 11:52 [PATCH 07/17] netconfig: Use an internal rtnl socket for l_netconfig_apply_rtnl Andrew Zaborowski
-- strict thread matches above, loose matches on Subject: below --
2022-05-19 21:38 Andrew Zaborowski
2022-05-19 21:28 Denis Kenzior
2022-05-19 20:53 Andrew Zaborowski
2022-05-19 20:28 Denis Kenzior
2022-05-18 18:58 Denis Kenzior
2022-05-13 22:47 Andrew Zaborowski
2022-05-13 14:55 Andrew Zaborowski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).