[2/3] Add new function ip_get__assigned_addresses_from_net()

Message ID 1519383935-3556-2-git-send-email-jonatan.schlag@ipfire.org
State New
Headers
Series [1/3] Add new function: device_get_by_assigned_ip_address() |

Commit Message

Jonatan Schlag via network Feb. 23, 2018, 10:05 p.m. UTC
  This function is neede by IPsec to set the routes correctly.
We can now now find a source IP for a given net.
This way is ugly because the source IP
is unpredictable if we get multiple IPs.

Signed-off-by: Jonatan Schlag <jonatan.schlag@ipfire.org>
---
 src/functions/functions.ip | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)
  

Comments

Jonatan Schlag via network Feb. 24, 2018, 10:53 p.m. UTC | #1
On Fri, 2018-02-23 at 11:05 +0000, Jonatan Schlag via network wrote:
> This function is neede by IPsec to set the routes correctly.
> We can now now find a source IP for a given net.
> This way is ugly because the source IP
> is unpredictable if we get multiple IPs.
> 
> Signed-off-by: Jonatan Schlag <jonatan.schlag@ipfire.org>
> ---
>  src/functions/functions.ip | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/src/functions/functions.ip b/src/functions/functions.ip
> index 3b43da7..ef40bcc 100644
> --- a/src/functions/functions.ip
> +++ b/src/functions/functions.ip
> @@ -205,3 +205,28 @@ ip_address_del() {
>  
>  	return ${EXIT_OK}
>  }
> +
> +# Get all currently assigned addresse for a given network
> +ip_get_assigned_addresses_from_net() {
> +	local net=${1}
> +	shift
> +	local args="$@"
> +
> +	assert ip_net_is_valid ${net}

I think this assertion isn't needed because "ip" will check this and just throw
an error.

> +	local line
> +	local ips

It would be nicer if the "ips" variable would be called "addresses" because that
is the term that we actually use most of the time.

> +	# We read the output of $(ip addr show to ${net} ${args})
> +	while read -r line; do
> +		# We are only interested in lines which start with inet or
> inet6
> +		[[ "${line}" =~ ^(inet6 |inet ) ]] || continue
> +
> +		# We need the second word the line
> +		line=(${line})
> +		list_append "ips" "$(ip_split_prefix "${line[1]}")"

You could also achieve this by passing the line argument up to the first space
and use the "%" and "#" parameters in the brackets.

I am not sure if the conversion to the array has any implications.

> +	done <<< "$(ip addr show to "${net}" ${args})"
> +
> +	# We sort the list to get the lowest IP as first item
> +	print "$(list_sort ${ips})"

You don't need to call print here. This will create a subshell for the list_sort
call, but list_sort already prints the output, so you can just write:

  list_sort ${ips}

That will be a lot faster.

> +}

-Michael
  
Jonatan Schlag via network Feb. 28, 2018, 2:03 a.m. UTC | #2
Hi,

Am Sa, 24. Feb, 2018 um 12:53 schrieb Michael Tremer 
<michael.tremer@ipfire.org>:
> On Fri, 2018-02-23 at 11:05 +0000, Jonatan Schlag via network wrote:
>>  This function is neede by IPsec to set the routes correctly.
>>  We can now now find a source IP for a given net.
>>  This way is ugly because the source IP
>>  is unpredictable if we get multiple IPs.
>> 
>>  Signed-off-by: Jonatan Schlag <jonatan.schlag@ipfire.org>
>>  ---
>>   src/functions/functions.ip | 25 +++++++++++++++++++++++++
>>   1 file changed, 25 insertions(+)
>> 
>>  diff --git a/src/functions/functions.ip b/src/functions/functions.ip
>>  index 3b43da7..ef40bcc 100644
>>  --- a/src/functions/functions.ip
>>  +++ b/src/functions/functions.ip
>>  @@ -205,3 +205,28 @@ ip_address_del() {
>> 
>>   	return ${EXIT_OK}
>>   }
>>  +
>>  +# Get all currently assigned addresse for a given network
>>  +ip_get_assigned_addresses_from_net() {
>>  +	local net=${1}
>>  +	shift
>>  +	local args="$@"
>>  +
>>  +	assert ip_net_is_valid ${net}
> 
> I think this assertion isn't needed because "ip" will check this and 
> just throw
> an error.

But can we catch the error in a nice way? I could have a look at this 
but using $? should be harder then to use our normal functions.

> 
>>  +	local line
>>  +	local ips
> 
> It would be nicer if the "ips" variable would be called "addresses" 
> because that
> is the term that we actually use most of the time.

Ok

> 
> 
>>  +	# We read the output of $(ip addr show to ${net} ${args})
>>  +	while read -r line; do
>>  +		# We are only interested in lines which start with inet or
>>  inet6
>>  +		[[ "${line}" =~ ^(inet6 |inet ) ]] || continue
>>  +
>>  +		# We need the second word the line
>>  +		line=(${line})
>>  +		list_append "ips" "$(ip_split_prefix "${line[1]}")"
> 
> You could also achieve this by passing the line argument up to the 
> first space
> and use the "%" and "#" parameters in the brackets.
> 
> I am not sure if the conversion to the array has any implications.

So how can we make shure that this has no implications?

> 
> 
>>  +	done <<< "$(ip addr show to "${net}" ${args})"
>>  +
>>  +	# We sort the list to get the lowest IP as first item
>>  +	print "$(list_sort ${ips})"
> 
> You don't need to call print here. This will create a subshell for 
> the list_sort
> call, but list_sort already prints the output, so you can just write:
> 
>   list_sort ${ips}
> 
> That will be a lot faster.

Ok

> 
> 
>>  +}
> 
> -Michael
Hi,<br><br>Am Sa, 24. Feb, 2018 um 12:53  schrieb Michael Tremer &lt;michael.tremer@ipfire.org&gt;:<br>
<blockquote type="cite"><div class="plaintext" style="white-space: pre-wrap;">On Fri, 2018-02-23 at 11:05 +0000, Jonatan Schlag via network wrote:
<blockquote> This function is neede by IPsec to set the routes correctly.
 We can now now find a source IP for a given net.
 This way is ugly because the source IP
 is unpredictable if we get multiple IPs.
 
 Signed-off-by: Jonatan Schlag &lt;<a href="mailto:jonatan.schlag@ipfire.org">jonatan.schlag@ipfire.org</a>&gt;
 ---
  src/functions/functions.ip | 25 +++++++++++++++++++++++++
  1 file changed, 25 insertions(+)
 
 diff --git a/src/functions/functions.ip b/src/functions/functions.ip
 index 3b43da7..ef40bcc 100644
 --- a/src/functions/functions.ip
 +++ b/src/functions/functions.ip
 @@ -205,3 +205,28 @@ ip_address_del() {
  
  	return ${EXIT_OK}
  }
 +
 +# Get all currently assigned addresse for a given network
 +ip_get_assigned_addresses_from_net() {
 +	local net=${1}
 +	shift
 +	local args="$@"
 +
 +	assert ip_net_is_valid ${net}
</blockquote>
I think this assertion isn't needed because "ip" will check this and just throw
an error.
</div></blockquote><div><br></div><div>But can we catch the error in a nice way? I could have a look at this but using $? should be harder then to use our normal functions.&nbsp;</div><div><br></div><blockquote type="cite"><div class="plaintext" style="white-space: pre-wrap;">
<blockquote> +	local line
 +	local ips
</blockquote>
It would be nicer if the "ips" variable would be called "addresses" because that
is the term that we actually use most of the time.</div></blockquote><div><br></div>Ok<div><br><blockquote type="cite"><div class="plaintext" style="white-space: pre-wrap;">

<blockquote> +	# We read the output of $(ip addr show to ${net} ${args})
 +	while read -r line; do
 +		# We are only interested in lines which start with inet or
 inet6
 +		[[ "${line}" =~ ^(inet6 |inet ) ]] || continue
 +
 +		# We need the second word the line
 +		line=(${line})
 +		list_append "ips" "$(ip_split_prefix "${line[1]}")"
</blockquote>
You could also achieve this by passing the line argument up to the first space
and use the "%" and "#" parameters in the brackets.

I am not sure if the conversion to the array has any implications.</div></blockquote><div><br></div><div>So how can we make shure that this has no&nbsp;<span style="white-space: pre-wrap;">implications?</span></div><div><br></div><blockquote type="cite"><div class="plaintext" style="white-space: pre-wrap;">

<blockquote> +	done &lt;&lt;&lt; "$(ip addr show to "${net}" ${args})"
 +
 +	# We sort the list to get the lowest IP as first item
 +	print "$(list_sort ${ips})"
</blockquote>
You don't need to call print here. This will create a subshell for the list_sort
call, but list_sort already prints the output, so you can just write:

  list_sort ${ips}

That will be a lot faster.</div></blockquote><div><br></div>Ok<div><br><blockquote type="cite"><div class="plaintext" style="white-space: pre-wrap;">

<blockquote> +}
</blockquote>
-Michael
</div></blockquote></div></div>
  

Patch

diff --git a/src/functions/functions.ip b/src/functions/functions.ip
index 3b43da7..ef40bcc 100644
--- a/src/functions/functions.ip
+++ b/src/functions/functions.ip
@@ -205,3 +205,28 @@  ip_address_del() {
 
 	return ${EXIT_OK}
 }
+
+# Get all currently assigned addresse for a given network
+ip_get_assigned_addresses_from_net() {
+	local net=${1}
+	shift
+	local args="$@"
+
+	assert ip_net_is_valid ${net}
+
+	local line
+	local ips
+
+	# We read the output of $(ip addr show to ${net} ${args})
+	while read -r line; do
+		# We are only interested in lines which start with inet or inet6
+		[[ "${line}" =~ ^(inet6 |inet ) ]] || continue
+
+		# We need the second word the line
+		line=(${line})
+		list_append "ips" "$(ip_split_prefix "${line[1]}")"
+	done <<< "$(ip addr show to "${net}" ${args})"
+
+	# We sort the list to get the lowest IP as first item
+	print "$(list_sort ${ips})"
+}