[platform][marvell] Arm 32-bit Arch support changes #5749

Sabareesh-Kumar-Anandan · 2020-10-30T06:17:46Z

Signed-off-by: Sabareesh Kumar Anandan sanandan@marvell.com

- Why I did it
- How I did it
Added Arm 32-bit arch build fixes
Added marvell armhf platform specific changes

- How to verify it
Compiled for arm 32-bit sonic image and verified it

- Which release branch to backport (provide reason below if selected)

201811
201911
202006

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

platform/marvell-armhf/platform.conf

lguohan · 2020-10-30T16:06:44Z

platform/marvell-armhf/sonic-platform-et6448m/et6448m_plt_setup.sh

@@ -52,6 +52,9 @@ main()
 {
    fw_uboot_env_cfg
    et6448m_profile
+
+    python /etc/entropy.py &
+    /bin/sh /etc/inband_mgmt.sh


what is this inband mgmt port?

ET6448M board, down't have OOB (mgmt) port.
To access management, we have enabled in-band management feature in one of the data port.

lguohan · 2020-10-30T16:07:47Z

platform/marvell-armhf/sonic-platform-et6448m/inband_mgmt.sh

+           ifconfig eth0 down
+           systemctl restart networking
+       fi
+       sleep 120


such sleep concerns me. why do we need so long wait time.

once ip address is assigned for in-band port, we can add longer delay to check back the status of the port again

what ip addr here? are we assume dhcp on eth0? why do we need to wait for ip address to be assign on eth0?

In the same board, via Ethernet USB dongle, management interface can also be created. so one difference would be ip address would not be assigned for in-band interface as it would have been created after syncd init.
Whereas for external eth0, ip address would be assigned statically/dhcp, and it should not be disturbed unnecessarily.

lguohan · 2020-10-30T16:07:55Z

platform/marvell-armhf/sonic-platform-et6448m/inband_mgmt.sh

+     modprobe i2c_mv64xxx
+     modprobe i2c-dev
+     modprobe i2c_mux_gpio
+     sleep 60


such sleep concerns me. why do we need so long wait time.

Here, the logic is to start a monitoring loop for the creation of in-band management port once data ports are initialized through sai.
So sleep has been introduced to start later after syncd docker is initialized.

lguohan · 2020-10-30T16:08:17Z

platform/marvell-armhf/sonic-platform-et6448m/inband_mgmt.sh

+       fi
+       sleep 120
+   else
+     sleep 3


why sleep 3 here?

At any time user can restart syncd docker by any means, at that time this loop has to monitor the reincarnation of the data port used for in-band and then restart ifup scripts for management port.
Longer the delay lesser the cpu utilization.

lguohan · 2020-11-02T09:44:57Z

platform/marvell-armhf/sonic-platform-et6448m/et6448m_plt_setup.sh

@@ -52,6 +52,9 @@ main()
 {
    fw_uboot_env_cfg
    et6448m_profile
+
+    python /etc/entropy.py &


what does this entropy.py use for?

In A385 CPU arch, random number generator is not supported in CPU soc. During bootup, systemd requires lot of random seeds, so this has been handled in SW by providing entropy from pseudo random generator.

platform/marvell-armhf/sonic-platform-et6448m/entropy.py

qiluo-msft · 2020-11-13T18:43:50Z

platform/marvell-armhf/platform.conf

+fi
+# armhf-marvell_et6448m_52x-r0  - Platform = Et6448M
+# armhf-nokia_ixs7215_52x-r0 - Platform = Nokia IPD6448M
+PLATFORM=`sed -n 's/onie_platform=\(.*\)/\1/p' $MACH_FILE`


s/onie_platform=(.*)/\1/p [](start = 18, length = 27)

s/onie_platform=(.*)/\1/p [](start = 18, length = 27)

s/^onie_platform=\(.*\)/\1/p
Could you add ^ at the beginning of the regex? #Closed

qiluo-msft · 2020-11-13T18:46:27Z

platform/marvell-armhf/platform.conf

@@ -68,6 +100,13 @@ prepare_boot_menu() {
        echo $FW_ENV_DEFAULT > /etc/fw_env.config
        echo "Using pre-configured uboot env"
    fi
+    if [ "$PLATFORM" = "armhf-nokia_ixs7215_52x-r0" ]; then
+        #FW_ENV_DEFAULT='/dev/mtd1 0x0000000 0x10000'


#FW_ENV_DEFAULT='/dev/mtd1 0x0000000 0x10000' [](start = 8, length = 45)

#FW_ENV_DEFAULT='/dev/mtd1 0x0000000 0x10000' [](start = 8, length = 45)

remove unused code? #Closed

qiluo-msft · 2020-11-13T18:49:51Z

platform/marvell-armhf/platform.conf

+    blk_dev="/dev/sda"
+    demo_part=$(sgdisk -p $blk_dev | grep -e "$demo_volume_label" -e "$legacy_volume_label" | awk '{print $1}')
+    # ONIE partition size 168MB
+    onie_part_size=168


168 [](start = 19, length = 3)

How do you determine this number? #Closed

If board is from manufacturing without sonic partition, then skip onie partition size as per platform partition layout

qiluo-msft · 2020-11-13T18:50:50Z

platform/marvell-armhf/platform.conf

+        echo "Error: Unable to create partition $demo_part on $blk_dev"
+        exit 1
+    }
+    partprobe || true


Seems wrong indentation level. #Closed

qiluo-msft · 2020-11-13T18:51:05Z

platform/marvell-armhf/platform.conf

+    # Make filesystem
+    mkfs.ext4 -L $demo_volume_label /dev/$demo_dev
+
+}


remove extra empty line #Closed

qiluo-msft · 2020-11-13T18:51:31Z

Remove extra empty line #Closed

Refers to: platform/marvell-armhf/platform.conf:269 in 8765059. [](commit_id = 8765059e8f0d4264c793aa6f31312edf852727bf, deletion_comment = False)

qiluo-msft · 2020-11-19T06:20:42Z

platform/marvell-armhf/sonic-platform-et6448m/entropy.py

@@ -0,0 +1,17 @@
+#!/usr/bin/python


python [](start = 11, length = 6)

Is this script compatible with python3? #Closed

yes and verified

qiluo-msft · 2020-11-19T06:21:21Z

platform/marvell-armhf/sonic-platform-et6448m/entropy.py

+            d = urnd.read(512)
+            t = struct.pack('ii', 4 * len(d), len(d)) + d
+            fcntl.ioctl(rnd, RNDADDENTROPY, t)
+    time.sleep(30)


time.sleep(30) [](start = 4, length = 14)

why sleep? #Closed

In A385 CPU arch, random number generator is not supported in CPU soc. During bootup, systemd requires lot of random seeds, so this has been handled in SW by providing entropy from pseudo random generator. Added sleep 30 to not to hog the CPU

qiluo-msft · 2020-11-19T06:21:34Z

platform/marvell-armhf/sonic-platform-et6448m/et6448m_plt_setup.sh

@@ -52,6 +52,9 @@ main()
 {
    fw_uboot_env_cfg
    et6448m_profile
+
+    python /etc/entropy.py &


& [](start = 27, length = 1)

why run in background? #Closed

periodically we need to update random

qiluo-msft · 2020-11-19T06:23:01Z

platform/marvell-armhf/sonic-platform-et6448m/inband_mgmt.sh

+     modprobe i2c_mv64xxx
+     modprobe i2c-dev
+     modprobe i2c_mux_gpio
+     sleep 60


sleep 60 [](start = 5, length = 8)

In general I am worried about the magic number sleep in all scripts. Do you want to achieve some collaboration between them? Is there any risk in future platforms? #Closed

There is no use to start inband mgmt script before entire system is ready. So we have added 60sec sleep
This is specific only to this platform. Not applicable for other platforms.

Thank you for the info. So my concern is "entire system is ready" < 60s, any risk here? If risk happens, what is the impact?

In reply to: 526620478 [](ancestors = 526620478)

There is no risk in that case. In-band mgmt port will be brought up after that.

qiluo-msft · 2020-11-19T06:26:28Z

sonic-slave-buster/Dockerfile.j2

 FROM multiarch/debian-debootstrap:armhf-buster
+COPY --from=qemu /usr/bin/qemu-arm-static /usr/bin


qemu /usr/bin/qemu-arm-static [](start = 12, length = 29)

Why copy a binary from another image? Seems a hack. #Closed

Not a hack, but to change the qemu version from 4 to 5, and qemu 4 has severe segfaults with golang

Could you achieve this goal by installing qemu-user-static package? I see version 5.0 https://packages.debian.org/buster-backports/qemu-user-static

In reply to: 526621660 [](ancestors = 526621660)

Couldnt able to install qemu from debian buster.
Error Logs:
Unpacking qemu-user-static (1:5.0-14~ bpo10+1) ...
dpkg (subprocess): unable to execute rm command for cleanup (rm): Too many levels of symbolic links
dpkg: error while cleaning up:
rm command for cleanup subprocess returned error exit status 2
Could not exec dpkg!
E: Sub-process /usr/bin/dpkg returned an error code (100)
E: Problem executing scripts DPkg::Post-Invoke 'rm -f /var/cache/apt/archives/.deb /var/cache/apt/archives/partial/.deb /var/cache/apt/*.bin || true'
E: Sub-process returned an error code
The command '/bin/sh -c apt-get install -y qemu-user-static=1:5.0-14~bpo10+1' returned a non-zero code: 100

Not familiar with this error message. I notice you are using dpkg directly. Did you try use apt-get. Please add something like below into /etc/apt/sources.list

deb [arch=amd64] http://debian-archive.trafficmanager.net/debian buster-backports main

I don't have an armhf dev environment, so please test it first.

In reply to: 530707840 [](ancestors = 530707840,526621660)

yes, I have already done that.
echo 'deb [arch=armhf] http://ftp.debian.org/debian buster-backports main' >> /etc/apt/sources.list
apt-get install -y qemu-user-static=5.0-14~ bpo10+1

Sorry, I see deb ... line, but I cannot find the second part.

apt-get install -y qemu-user-static=5.0-14~ bpo10+1

In reply to: 532104630 [](ancestors = 532104630,530707840,526621660)

I tried below lines and it was not working. got same errors as above. So, I didnt add this to the PR.
echo 'deb [arch=armhf] http://ftp.debian.org/debian buster-backports main' >> /etc/apt/sources.list
apt-get install -y qemu-user-static=5.0-14~ bpo10+1

Hi @qiluo-msft, @Sabareesh-Kumar-Anandan,

We actually can't upgrade the qemu-user-static by doing apt-get install -y qemu-user-static=5.0-14~ bpo10+1. Because the docker sonic-slave-buster is already running in the emulated arm environment, if you try to install qemu-user-static, it will pull the qemu-user-static binaries from the Debian repo for the arm arch and these binaries are to emulate something else from the arm host arch.

What we want is to update a qemu-arm-static binary emulating the arm client arch from the x86_64 host arch, copying the new version from multiarch/qemu-user-static:x86_64-arm-5.0.0-2 is a correct way to do this.

Thanks,
Benjamin

qiluo-msft · 2020-11-19T06:29:20Z

platform/marvell-armhf/platform.conf

+    if [ "$PLATFORM" = "armhf-marvell_et6448m_52x-r0" ]; then
+        trap_push "${onie_bin} umount /dev/ubi0_0|| true"
+        mount -t ubifs /dev/ubi0_0 $demo_mnt || {
+            echo "mount -t ubifs /dev/ubi0_0 $demo_mnt Failed"


Failed [](start = 55, length = 6)

If this is a failure, exit? #Closed

on failure, trap push will take care as script is enabled with -e

mount ... || { echo "..." } will always succeed. I see your message said "Failed", so I guess you need exit after echo.

In reply to: 526622606 [](ancestors = 526622606)

qiluo-msft · 2020-11-19T06:29:35Z

Remove extra empty line #Closed

Refers to: platform/marvell-armhf/platform.conf:267 in 8765059. [](commit_id = 8765059e8f0d4264c793aa6f31312edf852727bf, deletion_comment = False)

Sabareesh-Kumar-Anandan · 2020-11-24T17:32:11Z

Remove extra empty line

Refers to: platform/marvell-armhf/platform.conf:267 in a085fde. [](commit_id = a085fde, deletion_comment = False)

removed it

Sabareesh-Kumar-Anandan · 2020-11-25T13:36:00Z

Hi @lguohan,
We have answered/addressed all the comments. Do you have any more comments?
Also, Can you please retrigger the tests? Failures were not caused by this PR.

Signed-off-by: Sabareesh Kumar Anandan <sanandan@marvell.com>

qiluo-msft

LGTM. Please also check with other reviewers.

- Added Arm 32-bit arch build fixes - Added marvell armhf platform specific changes Signed-off-by: Sabareesh Kumar Anandan <sanandan@marvell.com>

lguohan reviewed Oct 30, 2020

View reviewed changes

platform/marvell-armhf/platform.conf Outdated Show resolved Hide resolved

lguohan reviewed Oct 30, 2020

View reviewed changes

lguohan reviewed Nov 2, 2020

View reviewed changes

platform/marvell-armhf/sonic-platform-et6448m/entropy.py Outdated Show resolved Hide resolved

antony-rheneus mentioned this pull request Nov 9, 2020

ARM arch support for SONIC #3271

Closed

qiluo-msft reviewed Nov 13, 2020

View reviewed changes

Sabareesh-Kumar-Anandan force-pushed the nokia_pr branch from 235eb09 to a085fde Compare November 17, 2020 10:58

sumukhatv requested a review from qiluo-msft November 18, 2020 20:11

qiluo-msft reviewed Nov 19, 2020

View reviewed changes

Sabareesh-Kumar-Anandan force-pushed the nokia_pr branch from 3a80614 to 8667ee8 Compare November 25, 2020 15:47

Sabareesh Kumar Anandan added 4 commits November 30, 2020 13:45

[platform][marvell] Arm 32-bit Arch support changes

09de6e9

Signed-off-by: Sabareesh Kumar Anandan <sanandan@marvell.com>

[platform][marvell] changed python version

fc67069

Signed-off-by: Sabareesh Kumar Anandan <sanandan@marvell.com>

[platform][marvell] Enabled apparmor

7fa1a0c

[platform][marvell] removed empty line

3f1ada1

Sabareesh Kumar Anandan added 3 commits November 30, 2020 13:45

[platform][marvell][arm] removed unused modules

73537d6

Signed-off-by: Sabareesh Kumar Anandan <sanandan@marvell.com>

[platform][marvell] Reduced kernel log level

a727461

Signed-off-by: Sabareesh Kumar Anandan <sanandan@marvell.com>

[platform][marvell] Exit after mount fs failure

097eaa1

Sabareesh-Kumar-Anandan force-pushed the nokia_pr branch from 8765059 to 097eaa1 Compare November 30, 2020 08:16

qiluo-msft approved these changes Dec 1, 2020

View reviewed changes

lguohan merged commit fe524c3 into sonic-net:master Dec 3, 2020

Sabareesh-Kumar-Anandan deleted the nokia_pr branch December 15, 2020 17:17

dflynn-Nokia mentioned this pull request Jan 12, 2021

[Nokia ixs7215] Add SW assist for platform entropy & fix inband mgmt #6417

Merged

4 tasks

		FROM multiarch/debian-debootstrap:armhf-buster
		COPY --from=qemu /usr/bin/qemu-arm-static /usr/bin

[platform][marvell] Arm 32-bit Arch support changes #5749

[platform][marvell] Arm 32-bit Arch support changes #5749

Conversation

Sabareesh-Kumar-Anandan commented Oct 30, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

antony-rheneus Nov 2, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Nov 13, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Nov 13, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Nov 13, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Nov 13, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Nov 13, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft commented Nov 13, 2020 • edited Loading

qiluo-msft Nov 19, 2020 • edited Loading

Choose a reason for hiding this comment

Sabareesh-Kumar-Anandan Nov 19, 2020 • edited Loading

Choose a reason for hiding this comment

qiluo-msft Nov 19, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Nov 19, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Nov 19, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Nov 19, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Sabareesh-Kumar-Anandan Nov 27, 2020 • edited Loading

Choose a reason for hiding this comment

qiluo-msft Nov 28, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Nov 19, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft commented Nov 19, 2020 • edited Loading

Sabareesh-Kumar-Anandan commented Nov 24, 2020

Sabareesh-Kumar-Anandan commented Nov 25, 2020

qiluo-msft left a comment

Choose a reason for hiding this comment

antony-rheneus Nov 2, 2020 •

edited

Loading

qiluo-msft Nov 13, 2020 •

edited

Loading

qiluo-msft Nov 13, 2020 •

edited

Loading

qiluo-msft Nov 13, 2020 •

edited

Loading

qiluo-msft Nov 13, 2020 •

edited

Loading

qiluo-msft Nov 13, 2020 •

edited

Loading

qiluo-msft commented Nov 13, 2020 •

edited

Loading

qiluo-msft Nov 19, 2020 •

edited

Loading

Sabareesh-Kumar-Anandan Nov 19, 2020 •

edited

Loading

qiluo-msft Nov 19, 2020 •

edited

Loading

qiluo-msft Nov 19, 2020 •

edited

Loading

qiluo-msft Nov 19, 2020 •

edited

Loading

qiluo-msft Nov 19, 2020 •

edited

Loading

Sabareesh-Kumar-Anandan Nov 27, 2020 •

edited

Loading

qiluo-msft Nov 28, 2020 •

edited

Loading

qiluo-msft Nov 19, 2020 •

edited

Loading

qiluo-msft commented Nov 19, 2020 •

edited

Loading