Skip to content
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.

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

Fix Compilation Errors in armv8a and fix issue with NOS image discovery #459

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
16 changes: 11 additions & 5 deletions build-config/make/images.make
Original file line number Diff line number Diff line change
Expand Up @@ -196,11 +196,17 @@ endif
sysroot-check: $(SYSROOT_CHECK_STAMP)
$(SYSROOT_CHECK_STAMP): $(PACKAGES_INSTALL_STAMPS)
$(Q) for file in $(SYSROOT_LIBS) ; do \
[ -r "$(DEV_SYSROOT)/lib/$$file" ] || { \
echo "ERROR: Missing SYSROOT_LIB: $$file" ; \
exit 1; } ; \
find $(DEV_SYSROOT)/lib -name $$file | xargs -i \
cp -av {} $(SYSROOTDIR)/lib/ || exit 1 ; \
if [ "$(ARCH)" == "arm64" ] || [ "$(ARCH)" == "x86_64" ] ; then \
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is x86_64 included here? x86_64 is currently building fine as it is with the new crosstool-ng.

It seems like this is only an arm64 issue. I'm probably missing something.

[ -r "$(DEV_SYSROOT)/lib64/$$file" ] || [ -r "$(DEV_SYSROOT)/lib/$$file" ] || { \
echo "ERROR: Missing SYSROOT_LIB: $$file" ; \
exit 1; } ; \
find $(DEV_SYSROOT)/lib64 $(DEV_SYSROOT)/lib -name $$file | xargs -i cp -av {} $(SYSROOTDIR)/lib/ || exit 1 ; \
else \
[ -r "$(DEV_SYSROOT)/lib/$$file" ] || { \
echo "ERROR: Missing SYSROOT_LIB: $$file" ; \
exit 1; } ; \
find $(DEV_SYSROOT)/lib -name $$file | xargs -i cp -av {} $(SYSROOTDIR)/lib/ || exit 1 ; \
fi; \
done
$(Q) for file in $(DEBUG_UTILS) ; do \
cp -av $$file $(SYSROOTDIR)/usr/bin || exit 1 ; \
Expand Down
6 changes: 6 additions & 0 deletions build-config/make/sysroot.make
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ $(SYSROOT_INIT_STAMP): $(TREE_STAMP)
$(Q) mkdir -p -v -m 0755 $(SYSROOTDIR)/dev
$(Q) mkdir -p -v $(SYSROOTDIR)/{sys,proc,tmp,etc,lib,mnt}
$(Q) mkdir -p -v $(SYSROOTDIR)/{var/log,usr/lib,usr/bin,usr/sbin,usr/share/locale,lib,mnt}
$(Q) cd $(SYSROOTDIR) && ln -s lib lib32 && ln -s lib lib64
$(Q) cd $(SYSROOTDIR)/usr && ln -s lib lib32 && ln -s lib lib64
$(Q) touch $@

# Development sysroot, used for compiling and linking user space
Expand All @@ -53,6 +55,10 @@ $(DEV_SYSROOT_INIT_STAMP): $(TREE_STAMP) | $(XTOOLS_BUILD_STAMP)
$(Q) echo "==== Preparing a new development sysroot ===="
$(Q) rm -rf $(DEV_SYSROOT)
$(Q) cp -a $$($(CROSSBIN)/$(CROSSPREFIX)gcc -print-sysroot) $(DEV_SYSROOT)
$(Q) rsync -rl $(XTOOLS_INSTALL_DIR)/$(TARGET)/$(TARGET)/lib $(DEV_SYSROOT)
ifeq ($(ARCH),$(filter $(ARCH),arm64 x86_64))
$(Q) rsync -rl $(XTOOLS_INSTALL_DIR)/$(TARGET)/$(TARGET)/lib64 $(DEV_SYSROOT)
endif
$(Q) chmod +w -R $(DEV_SYSROOT)
$(Q) touch $@

Expand Down
10 changes: 6 additions & 4 deletions build-config/scripts/onie-mk-iso.sh
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,12 @@ RECOVERY_ISO_IMAGE=${12}
echo "ERROR: Unable to read recovery config directory: $RECOVERY_CONF_DIR"
exit 1
}
[ -r "${GRUB_TARGET_LIB_I386_DIR}/biosdisk.mod" ] || {
echo "ERROR: Does not look like valid GRUB i386-pc library directory: $GRUB_TARGET_LIB_I386_DIR"
exit 1
}
if [ "$FIRMWARE_TYPE" = "auto" ] || [ "$FIRMWARE_TYPE" = "bios" ] ; then
[ -r "${GRUB_TARGET_LIB_I386_DIR}/biosdisk.mod" ] || {
echo "ERROR: Does not look like valid GRUB i386-pc library directory: $GRUB_TARGET_LIB_I386_DIR"
exit 1
}
fi
Copy link
Contributor

Choose a reason for hiding this comment

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

The onie-mk-iso.sh change looks OK to me. This is orthogonal to the above bit about lib32 / lib64. Let's make this a separate patch.

[ -x "${GRUB_HOST_BIN_I386_DIR}/grub-mkimage" ] || {
echo "ERROR: Does not look like valid GRUB i386-pc bin directory: $GRUB_HOST_BIN_I386_DIR"
exit 1
Expand Down
73 changes: 36 additions & 37 deletions rootconf/default/bin/discover
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#!/bin/sh
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this issue needs some more thought. Also this issue should be in a separate pull request from the lib32/lib64 issues as they are unrelated.

Correct me if I'm wrong, but here is my take on what is happening:

The sd_dhcp4() function attempts DHCPv4 on all interfaces, but stops when it gets a reply from one. Let's say you have two interfaces eth0 and eth1, both of which have a DHCP server (possibility different servers, or different configurations) - lets call the DHCP severs dhcp0 and dhcp1 respectively.

The sd_dhcp4() loop will stop after eth0 gets a DHCP reply from dhcp0. eth1 never gets a chance to talk with dhcp1.

In your case the desired DHCP configuration (the tftp server info) is only valid from dhcp1 -- for example the reply from dhcp0 does not contain the tftp server info.

It seems like the ONIE discovery needs to be per interface and the image waterfall driven all the way through before moving on to the next interface. Today the code is not quite right, where it first tries to gather all the DHCP info from all interfaces and then drive the waterfall.

The reason the code stops after the first interface has a valid reply is that what if both dhcp0 and dhcp1 specify a tftp server? Who wins?

All this code needs a bit of a rework and you have done most that here -- thanks!

A few comments:

  1. sd_static and sd_localfs should only happen once per discovery loop. These should not happen once per network interface. I think once these discoveries are complete we could launch exec_installer just for them.

  2. sd_dhcp6, sd_dhcp4, sd_mdns, sd_fallback and neigh_discovery happen once per interface. After all of these network discoveries are complete for an interface, launch exec_installer. This would allow each interface to receive DHCP replies from different servers, with potentially different image URL configuration.

It looks like you have done most of this already. The only change I would make is that the static and local file system discovery only happen once per "discovery loop", instead of once per interface. Otherwise looks pretty reasonable.


# Copyright (C) 2013-2014 Curt Brune <[email protected]>
# Copyright (C) 2016 Pankaj Bansal <[email protected]>
#
# SPDX-License-Identifier: GPL-2.0

Expand Down Expand Up @@ -33,21 +34,19 @@ onie_parms_file="${ONIE_RUN_DIR}/onie_parms_file.txt"
neigh_discovery()
{

intf_list=$(net_intf)
intf=$1
ping_cnt=3

for i in $intf_list ; do
# Wait for interface link-local addr to leave tentative state
cnt=300
while [ $cnt -gt 0 ] ; do
foo=$(ip addr show dev $i | grep tentative)
[ "$?" -ne 0 ] && break
cnt=$(( $cnt - 1 ))
sleep 0.1
done
ping6 -I $i -c $ping_cnt ff02::1 > /dev/null 2>&1
ping -I $i -w 2 -c $ping_cnt -q 255.255.255.255 > /dev/null 2>&1
# Wait for interface link-local addr to leave tentative state
cnt=300
while [ $cnt -gt 0 ] ; do
foo=$(ip addr show dev $intf | grep tentative)
[ "$?" -ne 0 ] && break
cnt=$(( $cnt - 1 ))
sleep 0.1
done
ping6 -I $intf -c $ping_cnt ff02::1 > /dev/null 2>&1
ping -I $intf -w 2 -c $ping_cnt -q 255.255.255.255 > /dev/null 2>&1

# Allow neighbor table to populate -- better way?
sleep 4
Expand Down Expand Up @@ -115,22 +114,19 @@ sd_dhcp6()
# DHCPv4 service discovery
sd_dhcp4()
{
intf_list=$(net_intf)
intf=$1
udhcp_args="$(udhcpc_args) -t 2 -T 2 -n"
tmp=

udhcp_request_opts=
for o in 7 43 54 66 67 72 114 125 ; do
udhcp_request_opts="$udhcp_request_opts -O $o"
done

# Initate DHCP request on every interface in the list. Stop after
# one works.
# Initate DHCP request on specified interface.

for i in $intf_list ; do
log_debug_msg "Trying DHCPv4 on interface: $i"
tmp=$(udhcpc $udhcp_args $udhcp_request_opts -i $i -s /lib/onie/udhcp4_sd) && break
tmp=
done
log_debug_msg "Trying DHCPv4 on interface: $intf"
tmp=$(udhcpc $udhcp_args $udhcp_request_opts -i $intf -s /lib/onie/udhcp4_sd)

onie_disco="${onie_disco}${tmp##*ONIE_PARMS:}"
if [ -n "$onie_disco" ] ; then
Expand Down Expand Up @@ -162,30 +158,33 @@ service_discovery()
# add/update $onie_disco
log_console_msg "Starting ONIE Service Discovery"
onie_disco=
sd_static && return
sd_localfs
sd_dhcp6
sd_dhcp4
sd_mdns
sd_fallback
sd_static $* && return
sd_localfs $*
sd_dhcp6 $*
sd_dhcp4 $*
sd_mdns $*
sd_fallback $*
}

delay=20

# Download and run installer entry point
while true ; do
/etc/init.d/networking.sh discover
/etc/init.d/syslogd.sh discover
service_discovery
log_debug_msg "onie_disco: $onie_disco"
neigh_discovery
intf_list=$(net_intf)
for i in $intf_list ; do
/etc/init.d/networking.sh "$i" discover
/etc/init.d/syslogd.sh discover
service_discovery "$i"
log_debug_msg "onie_disco: $onie_disco"
neigh_discovery
Copy link
Contributor

Choose a reason for hiding this comment

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

looks like neigh_discovery should be passed $i


rm -f /var/run/install.rc
cat $onie_neigh_file > $onie_parms_file
echo "$onie_disco" >> $onie_parms_file
sed -e 's/@@/ = /g' -e 's/##/\n/g' $onie_parms_file | logger -t discover -p ${syslog_onie}.info
exec_installer $onie_parms_file 2>&1 | tee $tee_log_file | logger -t onie-exec -p ${syslog_onie}.info
[ -r /var/run/install.rc ] && [ "$(cat /var/run/install.rc)" = "0" ] && exit 0
rm -f /var/run/install.rc
cat $onie_neigh_file > $onie_parms_file
echo "$onie_disco" >> $onie_parms_file
sed -e 's/@@/ = /g' -e 's/##/\n/g' $onie_parms_file | logger -t discover -p ${syslog_onie}.info
exec_installer $onie_parms_file 2>&1 | tee $tee_log_file | logger -t onie-exec -p ${syslog_onie}.info
[ -r /var/run/install.rc ] && [ "$(cat /var/run/install.rc)" = "0" ] && exit 0
done
# pause to avoid DoSing someone
log_info_msg "Sleeping for $delay seconds "
cnt=0
Expand Down
55 changes: 25 additions & 30 deletions rootconf/default/etc/init.d/networking.sh
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
# Copyright (C) 2013,2014,2016 Curt Brune <[email protected]>
# Copyright (C) 2014 david_yang <[email protected]>
# Copyright (C) 2013 Doron Tsur <[email protected]>
# Copyright (C) 2016 Pankaj Bansal <[email protected]>
#
# SPDX-License-Identifier: GPL-2.0

Expand Down Expand Up @@ -78,8 +79,6 @@ config_ethmgmt_fallback()

local prefix=16
local default_hn="onie-host"
local intf_counter=$1
shift
local intf=$1
shift

Expand Down Expand Up @@ -155,34 +154,29 @@ check_link_up()
# 4. Fall back to well known IP address
config_ethmgmt()
{
intf_list=$(net_intf)
intf_counter=0
intf=$1
shift
return_value=0

# Bring up all the interfaces for the subsequent methods.
for intf in $intf_list ; do
cmd_run ifconfig $intf up
params="$intf $*"
eval "result_${intf}=0"
check_link_up $intf || {
log_console_msg "${intf}: link down. Skipping configuration."
eval "result_${intf}=1"
continue
}
config_ethmgmt_static $params || \
config_ethmgmt_dhcp6 $params || \
config_ethmgmt_dhcp4 $params || \
config_ethmgmt_fallback $intf_counter $params || \
eval "result_${intf}=1"
intf_counter=$(( $intf_counter + 1))
done
for intf in $intf_list ; do
eval "curr_intf_result=\${result_${intf}}"
if [ "x$curr_intf_result" != "x0" ] ; then
log_console_msg "Failed to configure ${intf} interface"
return_value=1
fi
done
# Bring up the interface for the subsequent methods.
cmd_run ifconfig $intf up
params="$intf $*"
eval "result_${intf}=0"
check_link_up $intf || {
log_console_msg "${intf}: link down. Skipping configuration."
eval "result_${intf}=1"
return $return_value
}
config_ethmgmt_static $params || \
config_ethmgmt_dhcp6 $params || \
config_ethmgmt_dhcp4 $params || \
config_ethmgmt_fallback $params || \
eval "result_${intf}=1"
eval "curr_intf_result=\${result_${intf}}"
if [ "x$curr_intf_result" != "x0" ] ; then
log_console_msg "Failed to configure ${intf} interface"
return_value=1
fi
return $return_value
}

Expand All @@ -209,7 +203,8 @@ if [ "$1" = "start" ] ; then
fi
log_info_msg "Using $intf MAC address: $mac"
intf_counter=$(( $intf_counter + 1))
config_ethmgmt "$intf" $*
done
else
config_ethmgmt $*
fi

config_ethmgmt "$*"