Skip to content

[Mellanox] Use MAC from EEPROM for PortChannels and VLAN Interfaces #1793

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jul 23, 2018
Merged

[Mellanox] Use MAC from EEPROM for PortChannels and VLAN Interfaces #1793

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e4b16e2
Use MAC from EEPROM for PortChannels
andriymoroz-mlnx Jun 12, 2018
ca3704e
Use MAC from EEPROM in DEVICE_METADATA
andriymoroz-mlnx Jun 14, 2018
8f15fd7
Get MAC via decode-syseeprom
andriymoroz-mlnx Jun 26, 2018
38663f4
hw-management is now a service
andriymoroz-mlnx Jul 11, 2018
048db4d
Add error handling for MAC fetch process
andriymoroz-mlnx Jul 13, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion dockers/docker-teamd/start.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,12 @@ rm -rf $TEAMD_CONF_PATH
mkdir -p $TEAMD_CONF_PATH

SONIC_ASIC_TYPE=$(sonic-cfggen -y /etc/sonic/sonic_version.yml -v asic_type)
MAC_ADDRESS=$(ip link show eth0 | grep ether | awk '{print $2}')

if [ "$SONIC_ASIC_TYPE" == "mellanox" ]; then
MAC_ADDRESS=$(sonic-cfggen -d -v DEVICE_METADATA.localhost.mac)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we do sudo decode-syseeprom -m here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unfortunately no, it is not available in the containers...

else
MAC_ADDRESS=$(ip link show eth0 | grep ether | awk '{print $2}')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cat /sys/class/net/eth0/address

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MAC_ADDRESS=$(</sys/class/net/eth0/address)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the case under "else" getting MAC address from the "ip link show eth0" is the old code and used for other platforms
I can change it to what you suggested, but I am not able to test it on non-Mellanox boxes...

fi

# Align last byte
if [ "$SONIC_ASIC_TYPE" == "mellanox" -o "$SONIC_ASIC_TYPE" == "centec" ]; then
Expand Down
2 changes: 1 addition & 1 deletion files/build_templates/sonic_debian_extension.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ sudo LANG=C chroot $FILESYSTEM_ROOT systemctl enable hostname-config.service
sudo cp $IMAGE_CONFIGS/hostname/hostname-config.sh $FILESYSTEM_ROOT/usr/bin/

# Copy updategraph script and service file
sudo cp $IMAGE_CONFIGS/updategraph/updategraph.service $FILESYSTEM_ROOT/etc/systemd/system/
j2 files/build_templates/updategraph.service.j2 | sudo tee $FILESYSTEM_ROOT/etc/systemd/system/updategraph.service
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need to copy everything to the screen?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to get it in build log, I think
seen it in some other place
suggest to remove tee?

sudo LANG=C chroot $FILESYSTEM_ROOT systemctl enable updategraph.service
sudo cp $IMAGE_CONFIGS/updategraph/updategraph $FILESYSTEM_ROOT/usr/bin/
{% if enable_dhcp_graph_service == "y" %}
Expand Down
4 changes: 4 additions & 0 deletions ...ge_config/updategraph/updategraph.service → files/build_templates/updategraph.service.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ Requires=database.service

[Service]
Type=oneshot
{% if sonic_asic_platform == 'mellanox' -%}
EnvironmentFile=/host/machine.conf
ExecStartPre=/bin/bash -c "/usr/share/sonic/device/$onie_platform/hw-management start"
Copy link
Collaborator

@qiluo-msft qiluo-msft Jun 27, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hw-management [](start = 66, length = 13)

Is updategraph a good location for hw-management? It seems has nothing todo with graph. @taoyl-ms #Closed

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree with Qi. I don't see the reason to put hw-management here, and I don't see the need to change this into a j2 file either. Platform information can also be tested during runtime.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need drivers to access eeprom
j2 to avoid this code for others

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not create another service for this?
And, you can create a shell script to enable hw-management in which you can test platform information run time. It is much easier to follow than a build-time j2 rendering.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in current case, when will hw-management be started? Maybe hw-management itself should be a service and make it start early.

{% endif -%}
ExecStart=/usr/bin/updategraph
RemainAfterExit=yes

Expand Down
2 changes: 2 additions & 0 deletions slave.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -481,6 +481,8 @@ $(addprefix $(TARGET_PATH)/, $(SONIC_INSTALLERS)) : $(TARGET_PATH)/% : \
j2 -f env files/initramfs-tools/union-mount.j2 onie-image.conf > files/initramfs-tools/union-mount
j2 -f env files/initramfs-tools/arista-convertfs.j2 onie-image.conf > files/initramfs-tools/arista-convertfs

j2 files/build_templates/updategraph.service.j2 > updategraph.service
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this line necessary? The same operation is being done in sonic_debian_extension.j2 above and there the resulting file is getting copied to its proper final destination. Why is this file getting written to the repo root? It doesn't appear to be used anywhere.


$(if $($*_DOCKERS),
j2 files/build_templates/sonic_debian_extension.j2 > sonic_debian_extension.sh
chmod +x sonic_debian_extension.sh,
Expand Down
10 changes: 8 additions & 2 deletions src/sonic-config-engine/sonic_platform.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,18 @@ def get_sonic_version_info():
return data

def get_system_mac():
proc = subprocess.Popen("ip link show eth0 | grep ether | awk '{print $2}'", shell=True, stdout=subprocess.PIPE)
version_info = get_sonic_version_info()

if (version_info['asic_type'] == 'mellanox'):
get_mac_cmd = "sudo decode-syseeprom -m"
else:
get_mac_cmd = "ip link show eth0 | grep ether | awk '{print $2}'"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

read a file "/sys/class/net/eth0/address"?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

like above - not sure about other platforms


proc = subprocess.Popen(get_mac_cmd, shell=True, stdout=subprocess.PIPE)
(mac, err) = proc.communicate()
Copy link
Collaborator

@qiluo-msft qiluo-msft Jun 27, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

err [](start = 10, length = 3)

Handle error before using mac. #Closed

mac = mac.strip()

# Align last byte of MAC if necessary
version_info = get_sonic_version_info()
if version_info and (version_info['asic_type'] == 'mellanox' or version_info['asic_type'] == 'centec'):
last_byte = mac[-2:]
aligned_last_byte = format(int(int(last_byte, 16) & 0b11000000), '02x')
Expand Down