Send connman mailing list submissions to
        connman@lists.01.org

To subscribe or unsubscribe via the World Wide Web, visit
        https://lists.01.org/mailman/listinfo/connman
or, via email, send a message with subject or body 'help' to
        connman-requ...@lists.01.org

You can reach the person managing the list at
        connman-ow...@lists.01.org

When replying, please edit your Subject line so it is more specific
than "Re: Contents of connman digest..."


Today's Topics:

   1. Re: Planning Address Conflict Detection RFC-5227
      (Christian Spielberger)
   2. Re: Planning Address Conflict Detection RFC-5227 (Daniel Wagner)
   3. Re: Planning Address Conflict Detection RFC-5227
      (Christian Spielberger)
   4. Re: Planning Address Conflict Detection RFC-5227 (Daniel Wagner)


----------------------------------------------------------------------

Message: 1
Date: Mon, 12 Feb 2018 10:00:59 +0100
From: Christian Spielberger <christian.spielber...@gmail.com>
To: Daniel Wagner <w...@monom.org>
Cc: connman@lists.01.org
Subject: Re: Planning Address Conflict Detection RFC-5227
Message-ID: <20180212090058.gf28...@gmail.com>
Content-Type: text/plain; charset=us-ascii

On Fri 09.02.18  16:24, Daniel Wagner wrote:
> Hi Christian,
> 
> sorry for the late response due $reason...
> 
> On 07.02.2018 11:03, Christian Spielberger wrote:
> > Hello,
> > 
> > We are planning to add Address Conflict Detection RFC-5227 into connman. 
> 
> Nice :)
> 
> > It will
> > be my task to implement the main aspects of this RFC in the next 2-3 weeks. 
> > Now I will start by cloning the project to github. Then looking where and 
> > how
> > to place the ACD code. If you have any hints, please let me know!
> 
> I would start by looking what we have in gdhcp/client.c. There is some
> code in there for L2/L3 handling. From a quick glance in the RFC it
> looks like the right place to start.
> 
> > The goal would be definitely to bring it upstream.
> 
> That is highly appreciated!
> 
> Thanks,
> Daniel

Hi Daniel,

thanks for this hint! Indeed the ACD (RFC-5227) and IPv4ll (RFC-3927) seem
to have a copy paste relation. And the code for IPv4ll is useful for my
intention.

So, here more concrete questions:

- We could use the utility functions in gdhcp/ipv4ll.c for sending arp packets.
The functions ipv4ll_send_arp_packet and ipv4ll_arp_socket are not specific to
dhcp. Should I move them to another folder (src, or new arp) and rename them?

- As you stated, very useful is also gdhcp/client.c. The functions
send_probe_packet, send_announce_packet, and so on do exactly what we need for
ACD. But they are all related to dhcp because they use the structure 
GDHCPClient.
ACD should work for dhcp and static ipv4 configurations.
Do you want some refactoring? Clearly, the simplest way is to copy and rewrite
these functions and don't touch client.c. What do you think?

- Last question is where to trigger the ACD. I made a call-graph that shows
which functions lead to inet.c function
__connman_inet_modify_address
where the ip address is assigned to the interface. (Dot code below!) Maybe ACD
should be triggered in inet.c function
connman_inet_set_address ?
This also influences vpn-ipconfig.c and vpn-provider.c where
connman_inet_set_address is called. Or should I place it one step upward in
ipconfig.c function
__connman_ipconfig_address_add ?

regards,
Christian.

#####Call graph (use e.g. xdot to view) #############
digraph change_ipv4 {
        rankdir=LR
                node [shape = box]
                {
                        modAd [label="inet.c\n__connman_inet_modify_address"]
                                setAd [label="inet.c\nconnman_inet_set_address"]
                                C [label="bridge.c\n__connman_bridge_enable"]
                                D [label="peer.c\nstart_dhcp_server"]
                                E [label="tethering.c\nsetup_tun_interface"]
                                F 
[label="tethering.c\n__connman_tethering_set_enabled"]
                                G [label="provider.c\nset_connected"]
                                H [label="network.c\ndhcp_success"]
                                I [label="network.c\ndhcpv6_set_addresses"]
                                J [label="network.c\nset_connected_manual"]
                                K [label="network.c\nmanual_ipv6_set"]
                                L [label="peer.c\ndhcp_callback"]

                                addAd 
[label="ipconfig.c\n__connman_ipconfig_address_add"]
                                vpnAd 
[label="vpn-ipconfig.c\n__vpn_ipconfig_address_add"]
                                vpncAd 
[label="vpn-provider.c\nvpn_provider_change_address"]
                }
        setAd -> modAd
                C -> modAd
                D -> modAd
                E -> modAd
                F -> C

                addAd -> setAd
                vpnAd -> setAd
                vpncAd -> setAd

                G -> addAd
                H -> addAd
                I -> addAd
                J -> addAd
                K -> addAd
                L -> addAd
}
#####################################################



------------------------------

Message: 2
Date: Mon, 12 Feb 2018 10:58:24 +0100
From: Daniel Wagner <w...@monom.org>
To: Christian Spielberger <christian.spielber...@gmail.com>
Cc: connman@lists.01.org
Subject: Re: Planning Address Conflict Detection RFC-5227
Message-ID: <0d727516-fbf3-8968-9334-f5dd2beb2...@monom.org>
Content-Type: text/plain; charset=utf-8

Good Morning Christian,

> So, here more concrete questions:
> 
> - We could use the utility functions in gdhcp/ipv4ll.c for sending arp 
> packets.
> The functions ipv4ll_send_arp_packet and ipv4ll_arp_socket are not specific to
> dhcp. Should I move them to another folder (src, or new arp) and rename them?

Indeed the ipv4ll_ are not specific to dhcp but currently we just have
one user. If you need it in src, then yes please move it there.

> - As you stated, very useful is also gdhcp/client.c. The functions
> send_probe_packet, send_announce_packet, and so on do exactly what we need for
> ACD. But they are all related to dhcp because they use the structure 
> GDHCPClient.
> ACD should work for dhcp and static ipv4 configurations.
> Do you want some refactoring? Clearly, the simplest way is to copy and rewrite
> these functions and don't touch client.c. What do you think?

Please don't do code duplication because it is a maintenance nightmare
in the long run.

> - Last question is where to trigger the ACD. I made a call-graph that shows
> which functions lead to inet.c function
> __connman_inet_modify_address
> where the ip address is assigned to the interface. (Dot code below!) Maybe ACD
> should be triggered in inet.c function
> connman_inet_set_address ?
> This also influences vpn-ipconfig.c and vpn-provider.c where
> connman_inet_set_address is called. Or should I place it one step upward in
> ipconfig.c function
> __connman_ipconfig_address_add ?

I didn't look into ACD yet. So my comments might not make sense. So far
the assumption was, whenever ConnMan is not setting a static address it
will ask the dhcp subsystem (unless IPv6 autoconfig) to fetch the
configuration. That's also why ipv4ll lives in this directory. Are you
sure you need to have ACD separated from the current gdhcp code? The RFC
even speaks from a strong relationship to ipv4ll.

Maybe you can start with the current structure and see if it fits or
not. Don't hesitate to post early draft versions of the code if you need
feedback. It is always a bit difficult to give good advice without
looking at real code.

HTH!

Thanks,
Daniel


------------------------------

Message: 3
Date: Mon, 12 Feb 2018 13:35:40 +0100
From: Christian Spielberger <christian.spielber...@gmail.com>
To: Daniel Wagner <w...@monom.org>
Cc: connman@lists.01.org
Subject: Re: Planning Address Conflict Detection RFC-5227
Message-ID: <20180212123539.gg28...@gmail.com>
Content-Type: text/plain; charset=us-ascii

On Mon 12.02.18  10:58, Daniel Wagner wrote:
> Good Morning Christian,
> 
> > So, here more concrete questions:
> > 
> > - We could use the utility functions in gdhcp/ipv4ll.c for sending arp 
> > packets.
> > The functions ipv4ll_send_arp_packet and ipv4ll_arp_socket are not specific 
> > to
> > dhcp. Should I move them to another folder (src, or new arp) and rename 
> > them?
> 
> Indeed the ipv4ll_ are not specific to dhcp but currently we just have
> one user. If you need it in src, then yes please move it there.
> 

Hi Daniel,

Ok. Then I will move it to src.

> > - As you stated, very useful is also gdhcp/client.c. The functions
> > send_probe_packet, send_announce_packet, and so on do exactly what we need 
> > for
> > ACD. But they are all related to dhcp because they use the structure 
> > GDHCPClient.
> > ACD should work for dhcp and static ipv4 configurations.
> > Do you want some refactoring? Clearly, the simplest way is to copy and 
> > rewrite
> > these functions and don't touch client.c. What do you think?
> 
> Please don't do code duplication because it is a maintenance nightmare
> in the long run.
> 

Ok, then refactoring. Good choice!

> > - Last question is where to trigger the ACD. I made a call-graph that shows
> > which functions lead to inet.c function
> > __connman_inet_modify_address
> > where the ip address is assigned to the interface. (Dot code below!) Maybe 
> > ACD
> > should be triggered in inet.c function
> > connman_inet_set_address ?
> > This also influences vpn-ipconfig.c and vpn-provider.c where
> > connman_inet_set_address is called. Or should I place it one step upward in
> > ipconfig.c function
> > __connman_ipconfig_address_add ?
> 
> I didn't look into ACD yet. So my comments might not make sense. So far
> the assumption was, whenever ConnMan is not setting a static address it
> will ask the dhcp subsystem (unless IPv6 autoconfig) to fetch the
> configuration. That's also why ipv4ll lives in this directory. Are you
> sure you need to have ACD separated from the current gdhcp code? The RFC
> even speaks from a strong relationship to ipv4ll.
> 
> Maybe you can start with the current structure and see if it fits or
> not. Don't hesitate to post early draft versions of the code if you need
> feedback. It is always a bit difficult to give good advice without
> looking at real code.

Ok, I will try to make a proposition. But it has to be more general than only
for dhcp. Also for static ipv4. 

> 
> HTH!
> 
> Thanks,
> Daniel

Thanks,
Christian.


------------------------------

Message: 4
Date: Mon, 12 Feb 2018 13:48:15 +0100
From: Daniel Wagner <w...@monom.org>
To: Christian Spielberger <christian.spielber...@gmail.com>
Cc: connman@lists.01.org
Subject: Re: Planning Address Conflict Detection RFC-5227
Message-ID: <16ecd7fc-6b10-1c5c-44a3-bb0c38ddf...@monom.org>
Content-Type: text/plain; charset=utf-8

Hi Christian,

>> Maybe you can start with the current structure and see if it fits or
>> not. Don't hesitate to post early draft versions of the code if you need
>> feedback. It is always a bit difficult to give good advice without
>> looking at real code.
> 
> Ok, I will try to make a proposition. But it has to be more general than only
> for dhcp. Also for static ipv4. 

Ah, that explains it. Yes, then it makes sense to move the ipv4ll to src.

I should read the RFC :)

Thanks,
Daniel


------------------------------

Subject: Digest Footer

_______________________________________________
connman mailing list
connman@lists.01.org
https://lists.01.org/mailman/listinfo/connman


------------------------------

End of connman Digest, Vol 28, Issue 8
**************************************

Reply via email to