dhclient-script: fix bound

Message ID 1503926835-2098-1-git-send-email-jonatan.schlag@ipfire.org
State Accepted
Commit f5551504acee63fdb27511ae10418675640e05f4
Headers
Series dhclient-script: fix bound |

Commit Message

Jonatan Schlag Aug. 28, 2017, 11:27 p.m. UTC
  When we get a BOUND we should have only new_* variables set.
So it is stated in the manpage.
Apparently, also old_* variables are set so we never got into the block where the IP address was set.
We now always set a new IP Address when we get a BOUND.

Fixes: #11363

Signed-off-by: Jonatan Schlag <jonatan.schlag@ipfire.org>
---
 src/dhclient-script | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
  

Comments

Michael Tremer Sept. 8, 2017, 5:46 a.m. UTC | #1
Thanks for investigating this one. It was a tough one. Great work!

-Michael

On Mon, 2017-08-28 at 15:27 +0200, Jonatan Schlag wrote:
> When we get a BOUND we should have only new_* variables set.
> So it is stated in the manpage.
> Apparently, also old_* variables are set so we never got into the block where
> the IP address was set.
> We now always set a new IP Address when we get a BOUND.
> 
> Fixes: #11363
> 
> Signed-off-by: Jonatan Schlag <jonatan.schlag@ipfire.org>
> ---
>  src/dhclient-script | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/src/dhclient-script b/src/dhclient-script
> index c2e0678..ec990bf 100644
> --- a/src/dhclient-script
> +++ b/src/dhclient-script
> @@ -197,7 +197,8 @@ case "${reason}" in
>  
>  	BOUND|RENEW|REBIND|REBOOT)
>  		# Check if the IP address has changed. If so, delete all
> routes and stuff.
> -		if [ -n "${old_ip_address}" -a "${old_ip_address}" !=
> "${new_ip_address}" ]; then
> +		if [ -n "${old_ip_address}" -a "${old_ip_address}" !=
> "${new_ip_address}" ] || \
> +			[ "${reason}" = "BOUND" ]; then
>  			ipv4_flush_device ${interface}
>  		fi
>  
> @@ -208,7 +209,8 @@ case "${reason}" in
>  					[ ! "${old_network_number}" =
> "${new_network_number}" ] || \
>  					[ ! "${old_broadcast_address}" =
> "${new_broadcast_address}" ] || \
>  					[ ! "${old_routers}" =
> "${new_routers}" ] || \
> -					[ ! "${old_interface_mtu}" =
> "${new_interface_mtu}" ]; then
> +					[ ! "${old_interface_mtu}" =
> "${new_interface_mtu}" ] || \
> +					[ "${reason}" = "BOUND" ]; then
>  
>  
>  					# Calc the prefix from the subnet
> mask
  
Jonatan Schlag Jan. 21, 2018, 5:30 a.m. UTC | #2
Hi
Why was that patch not merged? I just searched for this patch and cloud 
not find him.

Am Mo, 28. Aug, 2017 um 3:27 schrieb Jonatan Schlag 
<jonatan.schlag@ipfire.org>:
> When we get a BOUND we should have only new_* variables set.
> So it is stated in the manpage.
> Apparently, also old_* variables are set so we never got into the 
> block where the IP address was set.
> We now always set a new IP Address when we get a BOUND.
> 
> Fixes: #11363
> 
> Signed-off-by: Jonatan Schlag <jonatan.schlag@ipfire.org>
> ---
>  src/dhclient-script | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/src/dhclient-script b/src/dhclient-script
> index c2e0678..ec990bf 100644
> --- a/src/dhclient-script
> +++ b/src/dhclient-script
> @@ -197,7 +197,8 @@ case "${reason}" in
> 
>  	BOUND|RENEW|REBIND|REBOOT)
>  		# Check if the IP address has changed. If so, delete all routes 
> and stuff.
> -		if [ -n "${old_ip_address}" -a "${old_ip_address}" != 
> "${new_ip_address}" ]; then
> +		if [ -n "${old_ip_address}" -a "${old_ip_address}" != 
> "${new_ip_address}" ] || \
> +			[ "${reason}" = "BOUND" ]; then
>  			ipv4_flush_device ${interface}
>  		fi
> 
> @@ -208,7 +209,8 @@ case "${reason}" in
>  					[ ! "${old_network_number}" = "${new_network_number}" ] || \
>  					[ ! "${old_broadcast_address}" = "${new_broadcast_address}" ] 
> || \
>  					[ ! "${old_routers}" = "${new_routers}" ] || \
> -					[ ! "${old_interface_mtu}" = "${new_interface_mtu}" ]; then
> +					[ ! "${old_interface_mtu}" = "${new_interface_mtu}" ] || \
> +					[ "${reason}" = "BOUND" ]; then
> 
> 
>  					# Calc the prefix from the subnet mask
> --
> 2.6.3
>
<div>Hi</div>Why was that patch not merged? I just searched for this patch and cloud not find him.<br><br>Am Mo, 28. Aug, 2017 um 3:27  schrieb Jonatan Schlag &lt;jonatan.schlag@ipfire.org&gt;:<br>
<blockquote type="cite"><div class="plaintext" style="white-space: pre-wrap;">When we get a BOUND we should have only new_* variables set.
So it is stated in the manpage.
Apparently, also old_* variables are set so we never got into the block where the IP address was set.
We now always set a new IP Address when we get a BOUND.

Fixes: #11363

Signed-off-by: Jonatan Schlag &lt;<a href="mailto:jonatan.schlag@ipfire.org">jonatan.schlag@ipfire.org</a>&gt;
---
 src/dhclient-script | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/dhclient-script b/src/dhclient-script
index c2e0678..ec990bf 100644
--- a/src/dhclient-script
+++ b/src/dhclient-script
@@ -197,7 +197,8 @@ case "${reason}" in
 
 	BOUND|RENEW|REBIND|REBOOT)
 		# Check if the IP address has changed. If so, delete all routes and stuff.
-		if [ -n "${old_ip_address}" -a "${old_ip_address}" != "${new_ip_address}" ]; then
+		if [ -n "${old_ip_address}" -a "${old_ip_address}" != "${new_ip_address}" ] || \
+			[ "${reason}" = "BOUND" ]; then
 			ipv4_flush_device ${interface}
 		fi
 
@@ -208,7 +209,8 @@ case "${reason}" in
 					[ ! "${old_network_number}" = "${new_network_number}" ] || \
 					[ ! "${old_broadcast_address}" = "${new_broadcast_address}" ] || \
 					[ ! "${old_routers}" = "${new_routers}" ] || \
-					[ ! "${old_interface_mtu}" = "${new_interface_mtu}" ]; then
+					[ ! "${old_interface_mtu}" = "${new_interface_mtu}" ] || \
+					[ "${reason}" = "BOUND" ]; then
 
 
 					# Calc the prefix from the subnet mask
<div>-- 
</div>2.6.3

</div></blockquote>
  
Michael Tremer Jan. 23, 2018, 12:08 a.m. UTC | #3
Hi,

I don't know why this hasn't been merged. It is a bit hacky and I cannot
remember if I wanted it to be reworked or so.

I merged it now because it is good enough for now and we probably need to
refactor the dhclient-script at some time.

Best,
-Michael

On Sat, 2018-01-20 at 19:30 +0100, Jonatan Schlag wrote:
> Hi
> Why was that patch not merged? I just searched for this patch and cloud not
> find him.
> 
> Am Mo, 28. Aug, 2017 um 3:27 schrieb Jonatan Schlag <jonatan.schlag@ipfire.org
> >:
> > When we get a BOUND we should have only new_* variables set.
> > So it is stated in the manpage.
> > Apparently, also old_* variables are set so we never got into the block
> > where the IP address was set.
> > We now always set a new IP Address when we get a BOUND.
> > 
> > Fixes: #11363
> > 
> > Signed-off-by: Jonatan Schlag <jonatan.schlag@ipfire.org>
> > ---
> >  src/dhclient-script | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/dhclient-script b/src/dhclient-script
> > index c2e0678..ec990bf 100644
> > --- a/src/dhclient-script
> > +++ b/src/dhclient-script
> > @@ -197,7 +197,8 @@ case "${reason}" in
> >  
> >  	BOUND|RENEW|REBIND|REBOOT)
> >  		# Check if the IP address has changed. If so, delete all
> > routes and stuff.
> > -		if [ -n "${old_ip_address}" -a "${old_ip_address}" !=
> > "${new_ip_address}" ]; then
> > +		if [ -n "${old_ip_address}" -a "${old_ip_address}" !=
> > "${new_ip_address}" ] || \
> > +			[ "${reason}" = "BOUND" ]; then
> >  			ipv4_flush_device ${interface}
> >  		fi
> >  
> > @@ -208,7 +209,8 @@ case "${reason}" in
> >  					[ ! "${old_network_number}" =
> > "${new_network_number}" ] || \
> >  					[ ! "${old_broadcast_address}" =
> > "${new_broadcast_address}" ] || \
> >  					[ ! "${old_routers}" =
> > "${new_routers}" ] || \
> > -					[ ! "${old_interface_mtu}" =
> > "${new_interface_mtu}" ]; then
> > +					[ ! "${old_interface_mtu}" =
> > "${new_interface_mtu}" ] || \
> > +					[ "${reason}" = "BOUND" ]; then
> >  
> >  
> >  					# Calc the prefix from the subnet
> > mask
> > -- 
> > 2.6.3
> >
  

Patch

diff --git a/src/dhclient-script b/src/dhclient-script
index c2e0678..ec990bf 100644
--- a/src/dhclient-script
+++ b/src/dhclient-script
@@ -197,7 +197,8 @@  case "${reason}" in
 
 	BOUND|RENEW|REBIND|REBOOT)
 		# Check if the IP address has changed. If so, delete all routes and stuff.
-		if [ -n "${old_ip_address}" -a "${old_ip_address}" != "${new_ip_address}" ]; then
+		if [ -n "${old_ip_address}" -a "${old_ip_address}" != "${new_ip_address}" ] || \
+			[ "${reason}" = "BOUND" ]; then
 			ipv4_flush_device ${interface}
 		fi
 
@@ -208,7 +209,8 @@  case "${reason}" in
 					[ ! "${old_network_number}" = "${new_network_number}" ] || \
 					[ ! "${old_broadcast_address}" = "${new_broadcast_address}" ] || \
 					[ ! "${old_routers}" = "${new_routers}" ] || \
-					[ ! "${old_interface_mtu}" = "${new_interface_mtu}" ]; then
+					[ ! "${old_interface_mtu}" = "${new_interface_mtu}" ] || \
+					[ "${reason}" = "BOUND" ]; then
 
 
 					# Calc the prefix from the subnet mask