Re: [systemd-devel] [PATCH] libnetworkd: only allow positive ifindex
Hi, On Sat, Apr 12, 2014 at 10:46 PM, Zbigniew Jędrzejewski-Szmek zbys...@in.waw.pl wrote: On Sat, Apr 12, 2014 at 09:59:52PM +0200, Tom Gundersen wrote: On Sat, Apr 12, 2014 at 7:26 PM, Zbigniew Jędrzejewski-Szmek zbys...@in.waw.pl wrote: On Sat, Apr 12, 2014 at 01:19:48PM +0200, Umut Tezduyar Lindskog wrote: Patch doesn't apply anymore after reference counting changes. I will re-send it along with ipv4ll tests which is coming up right away. Cool. Could you add a bit of a commit message explaining the change for the poor ignorant sods like me who don't know why indexes start at 0 or 1? We have already don't allow to start dhcp or ipv4ll with 0 or negative index (sd_ipv4ll_start, sd_dhcp_client_start). My patch only fixes the setter for the index. FWIW: the kernel starts enumerating interfaces at 1 [0]. Varies API's accept ifindex=0 to refer to unknown/any/all interfaces. That's not useful in these libraries though, as we only want to run these clients on a given interface, so restricting to ifindex 0 makes sense. -1 is a valid index but kernel will interpret it as the last dev in the array [0] Umut, will you resend with a bit more detailed commit message or should I fix it up when applying your patch? As a reminder, I have combined the content of this patch with http://lists.freedesktop.org/archives/systemd-devel/2014-April/018628.html (status: waiting) since I needed the correction for unit tests. If there will be reviews on the patch and I have to send an updated version, then I will write up more detailed message. Otherwise, could you please do it while taken the patch in. Thanks. Zbyszek [0]: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/net/core/dev.c#n5524 [0] - https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/net/core/dev.c#n194 Umut ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] libnetworkd: only allow positive ifindex
Patch doesn't apply anymore after reference counting changes. I will
re-send it along with ipv4ll tests which is coming up right away.
On Thu, Apr 10, 2014 at 2:35 PM, Umut Tezduyar Lindskog
umut.tezdu...@axis.com wrote:
From: Umut Tezduyar Lindskog umu...@axis.com
---
src/libsystemd-network/sd-dhcp-client.c |2 +-
src/libsystemd-network/sd-ipv4ll.c|2 +-
src/libsystemd-network/test-dhcp-client.c |4 +++-
3 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/libsystemd-network/sd-dhcp-client.c
b/src/libsystemd-network/sd-dhcp-client.c
index da41c47..70d1259 100644
--- a/src/libsystemd-network/sd-dhcp-client.c
+++ b/src/libsystemd-network/sd-dhcp-client.c
@@ -135,7 +135,7 @@ int sd_dhcp_client_set_request_address(sd_dhcp_client
*client,
int sd_dhcp_client_set_index(sd_dhcp_client *client, int interface_index) {
assert_return(client, -EINVAL);
assert_return(client-state == DHCP_STATE_INIT, -EBUSY);
-assert_return(interface_index = -1, -EINVAL);
+assert_return(interface_index 0, -EINVAL);
client-index = interface_index;
diff --git a/src/libsystemd-network/sd-ipv4ll.c
b/src/libsystemd-network/sd-ipv4ll.c
index 81fe85b..393a213 100644
--- a/src/libsystemd-network/sd-ipv4ll.c
+++ b/src/libsystemd-network/sd-ipv4ll.c
@@ -368,7 +368,7 @@ static int ipv4ll_receive_message(sd_event_source *s, int
fd,
int sd_ipv4ll_set_index(sd_ipv4ll *ll, int interface_index) {
assert_return(ll, -EINVAL);
-assert_return(interface_index = -1, -EINVAL);
+assert_return(interface_index 0, -EINVAL);
assert_return(ll-state == IPV4LL_STATE_INIT, -EBUSY);
ll-index = interface_index;
diff --git a/src/libsystemd-network/test-dhcp-client.c
b/src/libsystemd-network/test-dhcp-client.c
index a208b0d..3ba56b1 100644
--- a/src/libsystemd-network/test-dhcp-client.c
+++ b/src/libsystemd-network/test-dhcp-client.c
@@ -77,7 +77,9 @@ static void test_request_basic(sd_event *e)
assert_se(sd_dhcp_client_set_index(client, 15) == 0);
assert_se(sd_dhcp_client_set_index(client, -42) == -EINVAL);
-assert_se(sd_dhcp_client_set_index(client, -1) == 0);
+assert_se(sd_dhcp_client_set_index(client, -1) == -EINVAL);
+assert_se(sd_dhcp_client_set_index(client, 0) == -EINVAL);
+assert_se(sd_dhcp_client_set_index(client, 1) == 0);
assert_se(sd_dhcp_client_set_request_option(client,
DHCP_OPTION_SUBNET_MASK) == -EEXIST);
--
1.7.10.4
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] libnetworkd: only allow positive ifindex
On Sat, Apr 12, 2014 at 01:19:48PM +0200, Umut Tezduyar Lindskog wrote: Patch doesn't apply anymore after reference counting changes. I will re-send it along with ipv4ll tests which is coming up right away. Cool. Could you add a bit of a commit message explaining the change for the poor ignorant sods like me who don't know why indexes start at 0 or 1? Thanks :) Zbyszek ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] libnetworkd: only allow positive ifindex
On Sat, Apr 12, 2014 at 7:26 PM, Zbigniew Jędrzejewski-Szmek zbys...@in.waw.pl wrote: On Sat, Apr 12, 2014 at 01:19:48PM +0200, Umut Tezduyar Lindskog wrote: Patch doesn't apply anymore after reference counting changes. I will re-send it along with ipv4ll tests which is coming up right away. Cool. Could you add a bit of a commit message explaining the change for the poor ignorant sods like me who don't know why indexes start at 0 or 1? FWIW: the kernel starts enumerating interfaces at 1 [0]. Varies API's accept ifindex=0 to refer to unknown/any/all interfaces. That's not useful in these libraries though, as we only want to run these clients on a given interface, so restricting to ifindex 0 makes sense. Umut, will you resend with a bit more detailed commit message or should I fix it up when applying your patch? Cheers, Tom [0]: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/net/core/dev.c#n5524 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] libnetworkd: only allow positive ifindex
On Sat, Apr 12, 2014 at 09:59:52PM +0200, Tom Gundersen wrote: On Sat, Apr 12, 2014 at 7:26 PM, Zbigniew Jędrzejewski-Szmek zbys...@in.waw.pl wrote: On Sat, Apr 12, 2014 at 01:19:48PM +0200, Umut Tezduyar Lindskog wrote: Patch doesn't apply anymore after reference counting changes. I will re-send it along with ipv4ll tests which is coming up right away. Cool. Could you add a bit of a commit message explaining the change for the poor ignorant sods like me who don't know why indexes start at 0 or 1? FWIW: the kernel starts enumerating interfaces at 1 [0]. Varies API's accept ifindex=0 to refer to unknown/any/all interfaces. That's not useful in these libraries though, as we only want to run these clients on a given interface, so restricting to ifindex 0 makes sense. Umut, will you resend with a bit more detailed commit message or should I fix it up when applying your patch? Thanks. Zbyszek [0]: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/net/core/dev.c#n5524 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
