| Message ID | 20190911184807.21770-2-danieltimlee@gmail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | [v2,1/3] samples: pktgen: make variable consistent with option | expand |
On Thu, 12 Sep 2019 03:48:06 +0900 "Daniel T. Lee" <danieltimlee@gmail.com> wrote: > This commit adds CIDR parsing and IP validate helper function to parse > single IP or range of IP with CIDR. (e.g. 198.18.0.0/15) One question: You do know that this expansion of the CIDR will also include the CIDR network broadcast IP and "network-address", is that intentional?
On Fri, Sep 13, 2019 at 12:59 AM Jesper Dangaard Brouer <brouer@redhat.com> wrote: > > On Thu, 12 Sep 2019 03:48:06 +0900 > "Daniel T. Lee" <danieltimlee@gmail.com> wrote: > > > This commit adds CIDR parsing and IP validate helper function to parse > > single IP or range of IP with CIDR. (e.g. 198.18.0.0/15) > > One question: You do know that this expansion of the CIDR will also > include the CIDR network broadcast IP and "network-address", is that > intentional? > Correct. What I was trying to do with this script is, I want to test RSS/RPS and it does not really matters whether it is broadcast or network address, since the n-tuple hashing doesn't matter whether which kind of it. > -- > Best regards, > Jesper Dangaard Brouer > MSc.CS, Principal Kernel Engineer at Red Hat > LinkedIn: http://www.linkedin.com/in/brouer
On Fri, 13 Sep 2019 02:53:26 +0900 "Daniel T. Lee" <danieltimlee@gmail.com> wrote: > On Fri, Sep 13, 2019 at 12:59 AM Jesper Dangaard Brouer > <brouer@redhat.com> wrote: > > > > On Thu, 12 Sep 2019 03:48:06 +0900 > > "Daniel T. Lee" <danieltimlee@gmail.com> wrote: > > > > > This commit adds CIDR parsing and IP validate helper function to parse > > > single IP or range of IP with CIDR. (e.g. 198.18.0.0/15) > > > > One question: You do know that this expansion of the CIDR will also > > include the CIDR network broadcast IP and "network-address", is that > > intentional? > > > > Correct. > > What I was trying to do with this script is, > I want to test RSS/RPS and it does not > really matters whether it is broadcast or network address, > since the n-tuple hashing doesn't matter whether which kind of it. Okay, sounds valid to me. Some more feedback on the shell code... in another reply.
On Thu, 12 Sep 2019 03:48:06 +0900 "Daniel T. Lee" <danieltimlee@gmail.com> wrote: > This commit adds CIDR parsing and IP validate helper function to parse > single IP or range of IP with CIDR. (e.g. 198.18.0.0/15) > > Helpers will be used in prior to set target address in samples/pktgen. > > Signed-off-by: Daniel T. Lee <danieltimlee@gmail.com> > --- > samples/pktgen/functions.sh | 122 ++++++++++++++++++++++++++++++++++++ > 1 file changed, 122 insertions(+) > > diff --git a/samples/pktgen/functions.sh b/samples/pktgen/functions.sh > index 4af4046d71be..8be5a6b6c097 100644 [...] > +# Given a single IP(v4/v6) or CIDR, return minimum and maximum IP addr. > +function parse_addr() > +{ > + # check function is called with (funcname)6 > + [[ ${FUNCNAME[1]: -1} == 6 ]] && local IP6=6 > + local bitlen=$[ IP6 ? 128 : 32 ] > + local octet=$[ IP6 ? 16 : 8 ] > + > + local addr=$1 > + local net prefix > + local min_ip max_ip > + > + IFS='/' read net prefix <<< $addr > + [[ $IP6 ]] && net=$(extend_addr6 $net) > + validate_addr$IP6 $net > + > + if [[ $prefix -gt $bitlen ]]; then > + err 5 "Invalid prefix: $prefix" > + elif [[ -z $prefix ]]; then > + min_ip=$net > + max_ip=$net > + else > + # defining array for converting Decimal 2 Binary > + # 00000000 00000001 00000010 00000011 00000100 ... > + local d2b='{0..1}{0..1}{0..1}{0..1}{0..1}{0..1}{0..1}{0..1}' > + [[ $IP6 ]] && d2b+=$d2b > + eval local D2B=($d2b) I must say this is a rather cool shell/bash trick to use an array for converting decimal numbers into binary. > + > + local shift=$[ bitlen-prefix ] Using a variable named 'shift' is slightly problematic for shell/bash code. It works, but it is just confusing. > + local min_mask max_mask > + local min max > + local ip_bit > + local ip sep > + > + # set separator for each IP(v4/v6) > + [[ $IP6 ]] && sep=: || sep=. > + IFS=$sep read -ra ip <<< $net > + > + min_mask="$(printf '1%.s' $(seq $prefix))$(printf '0%.s' $(seq $shift))" > + max_mask="$(printf '0%.s' $(seq $prefix))$(printf '1%.s' $(seq $shift))" Also a surprising shell trick to get binary numbers out of a prefix number. > + > + # calculate min/max ip with &,| operator > + for i in "${!ip[@]}"; do > + digit=$[ IP6 ? 16#${ip[$i]} : ${ip[$i]} ] > + ip_bit=${D2B[$digit]} > + > + idx=$[ octet*i ] > + min[$i]=$[ 2#$ip_bit & 2#${min_mask:$idx:$octet} ] > + max[$i]=$[ 2#$ip_bit | 2#${max_mask:$idx:$octet} ] > + [[ $IP6 ]] && { min[$i]=$(printf '%X' ${min[$i]}); > + max[$i]=$(printf '%X' ${max[$i]}); } > + done > + > + min_ip=$(IFS=$sep; echo "${min[*]}") > + max_ip=$(IFS=$sep; echo "${max[*]}") > + fi > + > + echo $min_ip $max_ip > +} If you just fix the variable name 'shift' to something else, then I'm happy with this patch. Again, I'm very impressed with your shell/bash skills, I were certainly challenged when reviewing this :-)
On Fri, Sep 13, 2019 at 9:43 PM Jesper Dangaard Brouer <brouer@redhat.com> wrote: > > On Thu, 12 Sep 2019 03:48:06 +0900 > "Daniel T. Lee" <danieltimlee@gmail.com> wrote: > > > This commit adds CIDR parsing and IP validate helper function to parse > > single IP or range of IP with CIDR. (e.g. 198.18.0.0/15) > > > > Helpers will be used in prior to set target address in samples/pktgen. > > > > Signed-off-by: Daniel T. Lee <danieltimlee@gmail.com> > > --- > > samples/pktgen/functions.sh | 122 ++++++++++++++++++++++++++++++++++++ > > 1 file changed, 122 insertions(+) > > > > diff --git a/samples/pktgen/functions.sh b/samples/pktgen/functions.sh > > index 4af4046d71be..8be5a6b6c097 100644 > [...] > > > +# Given a single IP(v4/v6) or CIDR, return minimum and maximum IP addr. > > +function parse_addr() > > +{ > > + # check function is called with (funcname)6 > > + [[ ${FUNCNAME[1]: -1} == 6 ]] && local IP6=6 > > + local bitlen=$[ IP6 ? 128 : 32 ] > > + local octet=$[ IP6 ? 16 : 8 ] > > + > > + local addr=$1 > > + local net prefix > > + local min_ip max_ip > > + > > + IFS='/' read net prefix <<< $addr > > + [[ $IP6 ]] && net=$(extend_addr6 $net) > > + validate_addr$IP6 $net > > + > > + if [[ $prefix -gt $bitlen ]]; then > > + err 5 "Invalid prefix: $prefix" > > + elif [[ -z $prefix ]]; then > > + min_ip=$net > > + max_ip=$net > > + else > > + # defining array for converting Decimal 2 Binary > > + # 00000000 00000001 00000010 00000011 00000100 ... > > + local d2b='{0..1}{0..1}{0..1}{0..1}{0..1}{0..1}{0..1}{0..1}' > > + [[ $IP6 ]] && d2b+=$d2b > > + eval local D2B=($d2b) > > I must say this is a rather cool shell/bash trick to use an array for > converting decimal numbers into binary. > Thank you for the compliment and for the detailed review. > > + > > + local shift=$[ bitlen-prefix ] > > Using a variable named 'shift' is slightly problematic for shell/bash > code. It works, but it is just confusing. > > > + local min_mask max_mask > > + local min max > > + local ip_bit > > + local ip sep > > + > > + # set separator for each IP(v4/v6) > > + [[ $IP6 ]] && sep=: || sep=. > > + IFS=$sep read -ra ip <<< $net > > + > > + min_mask="$(printf '1%.s' $(seq $prefix))$(printf '0%.s' $(seq $shift))" > > + max_mask="$(printf '0%.s' $(seq $prefix))$(printf '1%.s' $(seq $shift))" > > Also a surprising shell trick to get binary numbers out of a prefix number. > > > + > > + # calculate min/max ip with &,| operator > > + for i in "${!ip[@]}"; do > > + digit=$[ IP6 ? 16#${ip[$i]} : ${ip[$i]} ] > > + ip_bit=${D2B[$digit]} > > + > > + idx=$[ octet*i ] > > + min[$i]=$[ 2#$ip_bit & 2#${min_mask:$idx:$octet} ] > > + max[$i]=$[ 2#$ip_bit | 2#${max_mask:$idx:$octet} ] > > + [[ $IP6 ]] && { min[$i]=$(printf '%X' ${min[$i]}); > > + max[$i]=$(printf '%X' ${max[$i]}); } > > + done > > + > > + min_ip=$(IFS=$sep; echo "${min[*]}") > > + max_ip=$(IFS=$sep; echo "${max[*]}") > > + fi > > + > > + echo $min_ip $max_ip > > +} > > If you just fix the variable name 'shift' to something else, then I'm > happy with this patch. > > Again, I'm very impressed with your shell/bash skills, I were certainly > challenged when reviewing this :-) > I'll change the variable name to 'remain'. Once again, I really appreciate your time and effort for the review. Thank you. Best, Daniel > -- > Best regards, > Jesper Dangaard Brouer > MSc.CS, Principal Kernel Engineer at Red Hat > LinkedIn: http://www.linkedin.com/in/brouer
diff --git a/samples/pktgen/functions.sh b/samples/pktgen/functions.sh index 4af4046d71be..8be5a6b6c097 100644 --- a/samples/pktgen/functions.sh +++ b/samples/pktgen/functions.sh @@ -163,6 +163,128 @@ function get_node_cpus() echo $node_cpu_list } +# Extend shrunken IPv6 address. +# fe80::42:bcff:fe84:e10a => fe80:0:0:0:42:bcff:fe84:e10a +function extend_addr6() +{ + local addr=$1 + local sep=: sep2=:: + local sep_cnt=$(tr -cd $sep <<< $1 | wc -c) + local shrink + + # separator count : should be between 2, 7. + if [[ $sep_cnt -lt 2 || $sep_cnt -gt 7 ]]; then + err 5 "Invalid IP6 address sep: $1" + fi + + # if shrink '::' occurs multiple, it's malformed. + shrink=( $(egrep -o "$sep{2,}" <<< $addr) ) + if [[ ${#shrink[@]} -ne 0 ]]; then + if [[ ${#shrink[@]} -gt 1 || ( ${shrink[0]} != $sep2 ) ]]; then + err 5 "Invalid IP$IP6 address shr: $1" + fi + fi + + # add 0 at begin & end, and extend addr by adding :0 + [[ ${addr:0:1} == $sep ]] && addr=0${addr} + [[ ${addr: -1} == $sep ]] && addr=${addr}0 + echo "${addr/$sep2/$(printf ':0%.s' $(seq $[8-sep_cnt])):}" +} + + +# Given a single IP(v4/v6) address, whether it is valid. +function validate_addr() +{ + # check function is called with (funcname)6 + [[ ${FUNCNAME[1]: -1} == 6 ]] && local IP6=6 + local len=$[ IP6 ? 8 : 4 ] + local max=$[ 2**(len*2)-1 ] + local addr sep + + # set separator for each IP(v4/v6) + [[ $IP6 ]] && sep=: || sep=. + IFS=$sep read -a addr <<< $1 + + # array length + if [[ ${#addr[@]} != $len ]]; then + err 5 "Invalid IP$IP6 address: $1" + fi + + # check each digit between 0, $max + for digit in "${addr[@]}"; do + [[ $IP6 ]] && digit=$[ 16#$digit ] + if [[ $digit -lt 0 || $digit -gt $max ]]; then + err 5 "Invalid IP$IP6 address: $1" + fi + done + + return 0 +} + +function validate_addr6() { validate_addr $@ ; } + +# Given a single IP(v4/v6) or CIDR, return minimum and maximum IP addr. +function parse_addr() +{ + # check function is called with (funcname)6 + [[ ${FUNCNAME[1]: -1} == 6 ]] && local IP6=6 + local bitlen=$[ IP6 ? 128 : 32 ] + local octet=$[ IP6 ? 16 : 8 ] + + local addr=$1 + local net prefix + local min_ip max_ip + + IFS='/' read net prefix <<< $addr + [[ $IP6 ]] && net=$(extend_addr6 $net) + validate_addr$IP6 $net + + if [[ $prefix -gt $bitlen ]]; then + err 5 "Invalid prefix: $prefix" + elif [[ -z $prefix ]]; then + min_ip=$net + max_ip=$net + else + # defining array for converting Decimal 2 Binary + # 00000000 00000001 00000010 00000011 00000100 ... + local d2b='{0..1}{0..1}{0..1}{0..1}{0..1}{0..1}{0..1}{0..1}' + [[ $IP6 ]] && d2b+=$d2b + eval local D2B=($d2b) + + local shift=$[ bitlen-prefix ] + local min_mask max_mask + local min max + local ip_bit + local ip sep + + # set separator for each IP(v4/v6) + [[ $IP6 ]] && sep=: || sep=. + IFS=$sep read -ra ip <<< $net + + min_mask="$(printf '1%.s' $(seq $prefix))$(printf '0%.s' $(seq $shift))" + max_mask="$(printf '0%.s' $(seq $prefix))$(printf '1%.s' $(seq $shift))" + + # calculate min/max ip with &,| operator + for i in "${!ip[@]}"; do + digit=$[ IP6 ? 16#${ip[$i]} : ${ip[$i]} ] + ip_bit=${D2B[$digit]} + + idx=$[ octet*i ] + min[$i]=$[ 2#$ip_bit & 2#${min_mask:$idx:$octet} ] + max[$i]=$[ 2#$ip_bit | 2#${max_mask:$idx:$octet} ] + [[ $IP6 ]] && { min[$i]=$(printf '%X' ${min[$i]}); + max[$i]=$(printf '%X' ${max[$i]}); } + done + + min_ip=$(IFS=$sep; echo "${min[*]}") + max_ip=$(IFS=$sep; echo "${max[*]}") + fi + + echo $min_ip $max_ip +} + +function parse_addr6() { parse_addr $@ ; } + # Given a single or range of port(s), return minimum and maximum port number. function parse_ports() {
This commit adds CIDR parsing and IP validate helper function to parse single IP or range of IP with CIDR. (e.g. 198.18.0.0/15) Helpers will be used in prior to set target address in samples/pktgen. Signed-off-by: Daniel T. Lee <danieltimlee@gmail.com> --- samples/pktgen/functions.sh | 122 ++++++++++++++++++++++++++++++++++++ 1 file changed, 122 insertions(+)