-
Notifications
You must be signed in to change notification settings - Fork 376
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
pankajbansal3073
wants to merge
2
commits into
opencomputeproject:master
Choose a base branch
from
pankajbansal3073:upstream
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
#!/bin/sh | ||
|
||
# Copyright (C) 2013-2014 Curt Brune <[email protected]> | ||
# Copyright (C) 2016 Pankaj Bansal <[email protected]> | ||
# | ||
# SPDX-License-Identifier: GPL-2.0 | ||
|
||
|
@@ -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 | ||
|
@@ -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 | ||
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. looks like |
||
|
||
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 | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
|
||
|
@@ -78,8 +79,6 @@ config_ethmgmt_fallback() | |
|
||
local prefix=16 | ||
local default_hn="onie-host" | ||
local intf_counter=$1 | ||
shift | ||
local intf=$1 | ||
shift | ||
|
||
|
@@ -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 | ||
} | ||
|
||
|
@@ -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 "$*" |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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 interfaceseth0
andeth1
, 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 aftereth0
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:
sd_static
andsd_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 launchexec_installer
just for them.sd_dhcp6
,sd_dhcp4
,sd_mdns
,sd_fallback
andneigh_discovery
happen once per interface. After all of these network discoveries are complete for an interface, launchexec_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.