[bfpxe] bfmac param not accepting MACs delimited with ":"

[pawel-baldysiak-dell]

Hi,
Currently "bfmac" param used in bfpxe is expecting to get:
bfmac=<mac>:<ip>:<netmask>
where MAC is in "aa-bb-cc-dd-ee-ff" format.
https://github.com/Mellanox/bfscripts/blob/master/bfpxe#L64C1-L67C43

I'm trying to have a dynamic grub.cfg file, where I can pass mac addresses via this variable.
The problem that I see is that grub is storing macs in "aa:bb:cc:dd:ee:ff" format - so if I create a param as:
bfmac=$net_default_mac:$net_default_ip:255.255.255.255
it would confuse the script due to too many ":" delimiters.

I was thinking about how to improve it and keep it backward compatible - came up with 3 ideas:

1. Change the way of paring each param:
   treat first 17 chars as MAC - change "-" to ":" no matter what is there
   Get param as second to last after ":"
   Get param as last after ":"

Code snippet for this:

realbfmac=$(echo $bfmac | cut -b1-17 | tr '-' ':')
ifname=$(grep $realbfmac /sys/class/net/*/address | awk -F\/ {'print $5'})
ifaddr=$(echo $bfmac | awk -F: '{print $(NF-1) }')
ifnet=$(echo $bfmac |  awk -F: '{print $(NF) }')

2. call tr to change potential ":" to "-" in first 17 bytes of bfmac:

Something like below + append rest of input

bfmac=$(echo $bfmac | cut -b1-17 | tr ':' '-')

3. get MAC as first 17 chars, then trim it from bfmac string

realbfmac=$(echo $bfmac | cut -b1-17 | tr '-' ':')
ifname=$(grep $realbfmac /sys/class/net/*/address | awk -F\/ {'print $5'})
bfmac=$(echo $bfmc | cut -b 18-)
ifaddr=$(echo $bfmac | cut -d: -f1)
ifnet=$(echo $bfmac |  cut -d: -f2)

Number 1 seems to be most elegant for me, while number 2 keeps the readability of the code - but calls /tr/ back and forth.

What's your opinion about those approaches? I could create a PR with fix once we agree on the options.

[mahantesh-nvidia]

I believe option 1 above is a better candidate to address backward compatibility and also handle grub format. This is efficient because the mac is required to be in the same format as grub stores is it, for finding the interface name.

[pawel-baldysiak-dell]

ok, thanks!
Will create a patch for that shortly

[pawel-baldysiak-dell]

Created PR to address this issue
#235

[mahantesh-nvidia]

Please assign reviewers (lsun100 and mahantesh-nvidia) to the PR. Thanks

[pawel-baldysiak-dell]

I'm afraid I can't do that :)
I don't have rights to assign reviewers