Re: [dpdk-dev] Performance issue in DPDK setup
> Is there a application available in DPDK where we can specify the sending > rate i.e. ask the application to send 1 Gbps traffic etc, so that it would > send 1 Gb traffic uniformly in the second. > Please guide us on this. > Hi The DPDK based Pkt-gen is doing a very good job for such uniform packet blasting http://dpdk.org/download http://pktgen.readthedocs.io/en/latest/ /Arnon
[dpdk-dev] releases scheduling
+1 for Ubuntu version numbering On Tue, Dec 15, 2015 at 3:37 PM, O'Driscoll, Tim wrote: > > > -Original Message- > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Thomas Monjalon > > Sent: Sunday, December 13, 2015 7:23 PM > > To: dev at dpdk.org > > Subject: [dpdk-dev] releases scheduling > > > > Hi all, > > > > We need to define the deadlines for the next releases. > > During 2015, we were doing a release every 4 months. > > If we keep the same pace, the next releases would be: > > 2.3: end of March > > 2.4: end of July > > 2.5: end of November > > > > However, things move fast and it may be a bit long to wait 4 months for > > a feature. That's why I suggest to progressively shorten release terms: > > 2.3: end of March > > 2.4: mid July > > 2.5: end of October > > and continue with a release every 3 months: > > 2.6: end of January > > 2.7: end of April > > 2.8: end of July > > This planning would preserve some of the major holiday periods > > (February, May, August, December). > > > > The first period, for the first submission of a feature, was 2 months > > long. > > Then we had 2 other months to discuss, merge and fix. > > We should shorten only the first period. > > > > Anyway, the next deadlines should be unchanged: > > - January 31: end of first submission phase > > - March 31: release 2.3 > > > > Opinions are welcome. > > I think moving to quarterly releases is a good idea. Your proposal to do > this in a gradual way, so that we don't change the 2.3 dates, also makes > sense. > > Should we consider changing the release numbering at the same time? It's > difficult to keep track of when each 2.x release is due, and we don't have > any criteria in place for moving to 3.x in future. Many people like the > Ubuntu numbering scheme of Year.Month. Should we consider adopting that > convention? If we move in future to a model where we have long-term support > releases (something that was touched on in Dublin), then we could append an > LTS designation to the release number. > > > Tim > -- *Arnon Warshavsky* *Qwilt | work: +972-72-2221634 | mobile: +972-50-8583058 | arnon at qwilt.com *
[dpdk-dev] tcpdump support in DPDK 2.3
we are thinking of making use > of > the multi-process infrastructure in DPDK. A secondary process can attach > to a > primary at runtime and provide the packet filtering and dumping > capability. > * ideally we want to create a generic packet mirroring callback inside the > EAL, > that can be set up to mirror packets going through Rx/Tx on an ethdev. > * using this, packets being received on the port to be monitored are sent > via > an rte_ring (ring ethdev) to the secondary process which takes those > packets > and does any filtering on them. [This would be where BPF could fit into > things, but it's not something we have looked at yet.] > * initially we plan to have the secondary process then write packets to a > pcap > file using a pcap PMD, but down the road if we get other PMDs, like a > KNI PMD > or a TAP device PMD, those could be used as targets instead. > > This implementation we hope should provide enough hooks to enable the > standard > tools to be used for monitoring and capturing packets. We will send out > draft > implementation code for various parts of this as soon as we have it. > > Additional feedback welcome, as always. :-) > > Regards, > /Bruce > > -- *Arnon Warshavsky* *Qwilt | work: +972-72-2221634 | mobile: +972-50-8583058 | arnon at qwilt.com *
[dpdk-dev] tcpdump support in DPDK 2.3
Filtering and serializing are 2 different components. No need to bind them by default, and nothing prevents you from calling them both from the same context if that what works for your use case. On Thu, Dec 17, 2015 at 1:38 AM, Matthew Hall wrote: > On Wed, Dec 16, 2015 at 11:45:46PM +0100, Morten Br?rup wrote: > > Matthew presented a very important point a few hours ago: We don't need > > tcpdump support for debugging the application in a lab; we already have > > plenty of other tools for debugging what we are developing. We need > tcpdump > > support for debugging network issues in a production network. > > +1 > > > In my "hardened network appliance" world, a solution designed purely for > > legacy applications (tcpdump, Wireshark etc.) is useless because the > network > > technician doesn't have access to these applications on the appliance. > > Maybe that's true on one exact system. But I've used a whole ton of systems > including appliances where this was not true. I really do want to find a > way > to support them, but according to my recent discussions w/ Alex Nasonov who > made bpfjit, I don't think it is possible without really tearing apart > libpcap. So for now the only good hope is Wireshark's Extcap support. > > > While a PC system running a DPDK based application might have plenty of > > spare lcores for filtering, the SmartShare appliances are already using > all > > lcores for dedicated purposes, so the runtime filtering has to be done by > > the IO lcores (otherwise we would have to rehash everything and > reallocate > > some lcores for mirroring, which I strongly oppose). Our non-DPDK > firmware > > has also always been filtering directly in the fast path. > > The shared process stuff and weird leftover lcore stuff seems way too > complex > for me whether or not there are any spare lcores. To me it seems easier if > I > just call some function and hand it mbufs, and it would quickly check them > against a linked list of active filters if filters are present, or do > nothing > and return if no filter is active. > > > If the filter is so complex that it unexpectedly degrades the normal > traffic > > forwarding performance > > If bpfjit is used, I think it is very hard to affect the performance much. > Unless you do something incredibly crazy. > > > Although it is generally considered bad design if a system's behavior (or > > performance) changes unexpectedly when debugging features are being used, > > I think we can keep the behavior change quite small using something like > what > I described. > > > Other companies might prefer to keep their fast path performance > unaffected > > and dedicate/reallocate some lcores for filtering. > > It always starts out unaffected... then goes back to accepting a bit of > slowness when people are forced to re-learn how bad it is with no > debugging. I > have seen it again and again in many companies. Hence my proposal for > efficient lightweight debugging support from the beginning. > > > 1. BPF filtering (... a DPDK variant of bpfjit), > > +1 > > > 2. scalable packet queueing for the mirrored packets (probably multi > > producer, single or multi consumer) > > I hate queueing. Queueing always reduces max possible throughput because > queueing is inefficient. It is better just to put them where they need to > go > immediately (run to completion) while the mbufs are already prefetched. > > > Then the DPDK application can take care of interfacing to > > the attached application and outputting the mirrored packets to the > > appropriate destination > > Too complicated. Pcap and extcap should be working by default. > > > A note about packet ordering: Mirrored packets belonging to different > flows > > are probably out of order because of RSS, where multiple lcores > contribute > > to the mirror output. > > Where I worry is weird configurations where a flow can occur in >1 cores. > But > I think most users try not to do this. > -- *Arnon Warshavsky* *Qwilt | work: +972-72-2221634 | mobile: +972-50-8583058 | arnon at qwilt.com *
[dpdk-dev] [PATCH v1] devtools: fix error propagation from check-forbidden-tokens.awk
Bugzilla ID: 165
Fixes: 4d4c612e6a30 ("devtools: check wrong svg include in guides")
Signed-off-by: Arnon Warshavsky
Explicitly collect the output and result of the
multiple awk script calls, print and return error
if any of them fails
---
devtools/checkpatches.sh | 21 +
1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
index ee8debe..df4336c 100755
--- a/devtools/checkpatches.sh
+++ b/devtools/checkpatches.sh
@@ -44,22 +44,35 @@ print_usage () {
}
check_forbidden_additions() { #
+ res=0
+
# refrain from new additions of rte_panic() and rte_exit()
# multiple folders and expressions are separated by spaces
- awk -v FOLDERS="lib drivers" \
+ result=$(awk -v FOLDERS="lib drivers" \
-v EXPRESSIONS="rte_panic\\\( rte_exit\\\(" \
-v RET_ON_FAIL=1 \
-v MESSAGE='Using rte_panic/rte_exit' \
-f $(dirname $(readlink -e $0))/check-forbidden-tokens.awk \
- "$1"
+ "$1")
+ if [ $? -ne 0 ] ; then
+ echo $result
+ res=1
+ fi
+
# svg figures must be included with wildcard extension
# because of png conversion for pdf docs
- awk -v FOLDERS='doc' \
+ result=$(awk -v FOLDERS='doc' \
-v EXPRESSIONS='::[[:space:]]*[^[:space:]]*\\.svg' \
-v RET_ON_FAIL=1 \
-v MESSAGE='Using explicit .svg extension instead of .*' \
-f $(dirname $(readlink -e $0))/check-forbidden-tokens.awk \
- "$1"
+ "$1")
+if [ $? -ne 0 ] ; then
+echo $result
+res=1
+fi
+
+ return $res
}
number=0
--
1.8.3.1
Re: [dpdk-dev] [PATCH v1] devtools: fix error propagation from check-forbidden-tokens.awk
The reason I did not use the && approach is that if both checks have errors, only the first will be reported and we want all errors to be reported at once without discovering them one by one after every fix.
Re: [dpdk-dev] [PATCH v1] devtools: fix error propagation from check-forbidden-tokens.awk
> > No need for all those checks on $? and the output saving. > You blew my cover guys. I am not a shell scripts person :) You are right. WIll change that
[dpdk-dev] [PATCH v2] devtools: fix error propagation from check-forbidden-tokens.awk
Bugzilla ID: 165
Fixes: 4d4c612e6a30 ("devtools: check wrong svg include in guides")
Signed-off-by: Arnon Warshavsky
Explicitly collect the error code of the
multiple awk script calls
---
devtools/checkpatches.sh | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
index ee8debe..3b03b7e 100755
--- a/devtools/checkpatches.sh
+++ b/devtools/checkpatches.sh
@@ -44,6 +44,8 @@ print_usage () {
}
check_forbidden_additions() { #
+ res=0
+
# refrain from new additions of rte_panic() and rte_exit()
# multiple folders and expressions are separated by spaces
awk -v FOLDERS="lib drivers" \
@@ -51,7 +53,8 @@ check_forbidden_additions() { #
-v RET_ON_FAIL=1 \
-v MESSAGE='Using rte_panic/rte_exit' \
-f $(dirname $(readlink -e $0))/check-forbidden-tokens.awk \
- "$1"
+ "$1" || res=1
+
# svg figures must be included with wildcard extension
# because of png conversion for pdf docs
awk -v FOLDERS='doc' \
@@ -59,7 +62,9 @@ check_forbidden_additions() { #
-v RET_ON_FAIL=1 \
-v MESSAGE='Using explicit .svg extension instead of .*' \
-f $(dirname $(readlink -e $0))/check-forbidden-tokens.awk \
- "$1"
+ "$1" || res = 1
+
+ return $res
}
number=0
--
1.8.3.1
Re: [dpdk-dev] Patch not seen in the Patchwork
If your patch arrived to your colleagues on the same domain but not to dev@dpdk, I would place a bet on your outgoing smtp server not sending it outside your domain.
Re: [dpdk-dev] [PATCH] devtools: check wrong svg include in patches
Hi Thomas
Glad the function gets to be reused :)
Now that it has more than one consumer apparently the function
check_forbidden_additions() cannot be fed by stdin.
I got it working with 3 changes:
1. call the awk script with $tmpinput as a parameter instead of stdin. This
aligns with Neils changes from previous version
2. Escaped the spaces from the regex , as the awk script uses spaces to
tell multiple forbidden expressions apart (which now I see sucks ...),
and added an extra backslash
3. call the function with no parameters (not needed anymore)
awk -v FOLDERS="lib drivers" \
-v EXPRESSIONS="rte_panic\\\( rte_exit\\\(" \
-v RET_ON_FAIL=1 \
- -f $(dirname $(readlink -e $0))/check-forbidden-tokens.awk -
+ -f $(dirname $(readlink -e $0))/check-forbidden-tokens.awk
*$tmpinput*
+
+ awk -v FOLDERS='doc' \
+ -v EXPRESSIONS='*::[[:space:]]*[^[:space:]]*\\\.svg*' \
+ -v RET_ON_FAIL=1 \
+ -f $(dirname $(readlink -e $0))/check-forbidden-tokens.awk
*$tmpinput*
! $verbose || printf '\nChecking forbidden tokens additions:\n'
- report=$(check_forbidden_additions *<"$tmpinput"*)
+ report=$(check_forbidden_additions)
thanks
/Arnon
Re: [dpdk-dev] [PATCH] devtools: check wrong svg include in patches
> > and added an extra backslash > > Are you sure we need an extra backslash? > Note that I am using single quotes. > I thought we need only 2 backslashes in this case. > You are right. Its only required with double quotes
Re: [dpdk-dev] [PATCH v2] devtools: check wrong svg include in guides
Hi
>PS: the warning contains the regex. May it be improved?
How about passing an explicit warning message to the awk instead of the
regex?
+++ b/devtools/check-forbidden-tokens.awk
END {
if (count > 0) {
print "Warning in " substr(last_file,6) ":"
- print "are you sure you want to add the following:"
- for (key in expressions) {
- if (expressions[key] > 0) {
- print key
- }
- }
+ print MESSAGE
exit RET_ON_FAIL
}
}
+++ b/devtools/checkpatches.sh
-check_forbidden_additions() {
+check_forbidden_additions() { #
+ local ret=0
# refrain from new additions of rte_panic() and rte_exit()
# multiple folders and expressions are separated by spaces
awk -v FOLDERS="lib drivers" \
-v EXPRESSIONS="rte_panic\\\( rte_exit\\\(" \
-v RET_ON_FAIL=1 \
- -f $(dirname $(readlink -e $0))/check-forbidden-tokens.awk -
+ -v MESSAGE='Usage of rte_panic/rte_exit' \
+ -f $(dirname $(readlink -e $0))/check-forbidden-tokens.awk \
+ "$1"
+ if [ $? -ne 0 ] ; then
+ret=1
+ fi
+ # svg figures must be included with wildcard extension
+ # because of png conversion for pdf docs
+ awk -v FOLDERS='doc' \
+ -v EXPRESSIONS='::[[:space:]]*[^[:space:]]*\\.svg' \
+ -v RET_ON_FAIL=1 \
+ -v MESSAGE='Using explicit .svg extention in figures
instead of .*' \
+ -f $(dirname $(readlink -e $0))/check-forbidden-tokens.awk \
+ "$1"
+ if [ $? -ne 0 ] ; then
+ret=1
+ fi
+
+ return $ret
}
I also noticed that there is a need to keep the return value from each
check in case the first fails and the latter succeeds
thanks
/Arnon
On Wed, Oct 31, 2018 at 6:28 PM, Thomas Monjalon
wrote:
> Including svg files with the svg extension is a common mistake:
> .. figure:: example.svg
> must be
> .. figure:: example.*
> So it will work also when building pdf doc with figures converted
> to png files.
>
> A check is added in checkpatches.sh.
>
> Signed-off-by: Thomas Monjalon
> Signed-off-by: Arnon Warshavsky
>
> PS: the warning contains the regex. May it be improved?
>
> Warning in /doc/guides/nics/mvpp2.rst:
> are you sure you want to add the following:
> ::[[:space:]]*[^[:space:]]*\.svg
>
> Cc: Arnon Warshavsky
> ---
> devtools/checkpatches.sh | 14 +++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
> index bf3114f95..fb9e9f76d 100755
> --- a/devtools/checkpatches.sh
> +++ b/devtools/checkpatches.sh
> @@ -43,13 +43,21 @@ print_usage () {
> END_OF_HELP
> }
>
> -check_forbidden_additions() {
> +check_forbidden_additions() { #
> # refrain from new additions of rte_panic() and rte_exit()
> # multiple folders and expressions are separated by spaces
> awk -v FOLDERS="lib drivers" \
> -v EXPRESSIONS="rte_panic\\\( rte_exit\\\(" \
> -v RET_ON_FAIL=1 \
> - -f $(dirname $(readlink -e $0))/check-forbidden-tokens.awk
> -
> + -f $(dirname $(readlink -e $0))/check-forbidden-tokens.awk
> \
> + "$1"
> + # svg figures must be included with wildcard extension
> + # because of png conversion for pdf docs
> + awk -v FOLDERS='doc' \
> + -v EXPRESSIONS='::[[:space:]]*[^[:space:]]*\\.svg' \
> + -v RET_ON_FAIL=1 \
> + -f $(dirname $(readlink -e $0))/check-forbidden-tokens.awk
> \
> + "$1"
> }
>
> number=0
> @@ -115,7 +123,7 @@ check () { #
> fi
>
> ! $verbose || printf '\nChecking forbidden tokens additions:\n'
> - report=$(check_forbidden_additions <"$tmpinput")
> + report=$(check_forbidden_additions "$tmpinput")
> if [ $? -ne 0 ] ; then
> $headline_printed || print_headline "$3"
> printf '%s\n' "$report"
> --
> 2.19.0
>
>
--
*Arnon Warshavsky*
*Qwilt | work: +972-72-2221634 | mobile: +972-50-8583058 | ar...@qwilt.com
*
Re: [dpdk-dev] [PATCH v2] devtools: check wrong svg include in guides
> > > Yes it is a good idea. > I think it can be a separate patch. Would you like to send it please? > > Sure. Will do
Re: [dpdk-dev] [PATCH v2] devtools: check wrong svg include in guides
> >> Yes it is a good idea. >> I think it can be a separate patch. Would you like to send it please? >> >> Sure. Will do > > Just to make sure - I am waiting for your patch to get in, so that I apply the warning patch for both checks -- *Arnon Warshavsky* *Qwilt | work: +972-72-2221634 | mobile: +972-50-8583058 | ar...@qwilt.com *
[dpdk-dev] [PATCH] devtools: add explicit warning messages for forbidden tokens
Replace the content of warning in the forbidden tokens script
from using the searched regex into using explicit messages
Signed-off-by: Arnon Warshavsky
---
devtools/check-forbidden-tokens.awk | 7 +--
devtools/checkpatches.sh| 3 +++
2 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/devtools/check-forbidden-tokens.awk
b/devtools/check-forbidden-tokens.awk
index fd77cdd..8c89de3 100755
--- a/devtools/check-forbidden-tokens.awk
+++ b/devtools/check-forbidden-tokens.awk
@@ -63,12 +63,7 @@ BEGIN {
END {
if (count > 0) {
print "Warning in " substr(last_file,6) ":"
- print "are you sure you want to add the following:"
- for (key in expressions) {
- if (expressions[key] > 0) {
- print key
- }
- }
+ print MESSAGE
exit RET_ON_FAIL
}
}
diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
index fb9e9f7..ef70229 100755
--- a/devtools/checkpatches.sh
+++ b/devtools/checkpatches.sh
@@ -49,13 +49,16 @@ check_forbidden_additions() { #
awk -v FOLDERS="lib drivers" \
-v EXPRESSIONS="rte_panic\\\( rte_exit\\\(" \
-v RET_ON_FAIL=1 \
+ -v MESSAGE='Using rte_panic/rte_exit' \
-f $(dirname $(readlink -e $0))/check-forbidden-tokens.awk \
"$1"
# svg figures must be included with wildcard extension
# because of png conversion for pdf docs
+ message="Using explicit .svg extention in figures instead of .*"
awk -v FOLDERS='doc' \
-v EXPRESSIONS='::[[:space:]]*[^[:space:]]*\\.svg' \
-v RET_ON_FAIL=1 \
+ -v MESSAGE="$message" \
-f $(dirname $(readlink -e $0))/check-forbidden-tokens.awk \
"$1"
}
--
1.8.3.1
[dpdk-dev] [PATCH v2] devtools: add explicit warning messages for forbidden tokens
Replace the content of warning in the forbidden tokens script
from using the searched regex into using explicit messages
Signed-off-by: Arnon Warshavsky
---
v2 - spelling typo
devtools/check-forbidden-tokens.awk | 7 +--
devtools/checkpatches.sh| 3 +++
2 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/devtools/check-forbidden-tokens.awk
b/devtools/check-forbidden-tokens.awk
index fd77cdd..8c89de3 100755
--- a/devtools/check-forbidden-tokens.awk
+++ b/devtools/check-forbidden-tokens.awk
@@ -63,12 +63,7 @@ BEGIN {
END {
if (count > 0) {
print "Warning in " substr(last_file,6) ":"
- print "are you sure you want to add the following:"
- for (key in expressions) {
- if (expressions[key] > 0) {
- print key
- }
- }
+ print MESSAGE
exit RET_ON_FAIL
}
}
diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
index fb9e9f7..40f4c26 100755
--- a/devtools/checkpatches.sh
+++ b/devtools/checkpatches.sh
@@ -49,13 +49,16 @@ check_forbidden_additions() { #
awk -v FOLDERS="lib drivers" \
-v EXPRESSIONS="rte_panic\\\( rte_exit\\\(" \
-v RET_ON_FAIL=1 \
+ -v MESSAGE='Using rte_panic/rte_exit' \
-f $(dirname $(readlink -e $0))/check-forbidden-tokens.awk \
"$1"
# svg figures must be included with wildcard extension
# because of png conversion for pdf docs
+ message="Using explicit .svg extension in figures instead of .*"
awk -v FOLDERS='doc' \
-v EXPRESSIONS='::[[:space:]]*[^[:space:]]*\\.svg' \
-v RET_ON_FAIL=1 \
+ -v MESSAGE="$message" \
-f $(dirname $(readlink -e $0))/check-forbidden-tokens.awk \
"$1"
}
--
1.8.3.1
Re: [dpdk-dev] [PATCH v2] devtools: add explicit warning messages for forbidden tokens
> > > + message="Using explicit .svg extension in figures instead of .*" > > awk -v FOLDERS='doc' \ > > -v EXPRESSIONS='::[[:space:]]*[^[:space:]]*\\.svg' \ > > -v RET_ON_FAIL=1 \ > > + -v MESSAGE="$message" \ > > -f $(dirname $(readlink -e $0))/check-forbidden-tokens.awk > \ > > "$1" > > } > > Why using a variable for message in the second check? > > > This was to avoid the 80 characters long line warning I get. It also seems more convenient should there be a need for multi-line messages. Is there a more preferred way in such a case of a passed parameter?
Re: [dpdk-dev] [PATCH v2] devtools: add explicit warning messages for forbidden tokens
> > > I think I prefer passing the string directly. > You can make a shorter message: > Using explicit .svg extension in rST instead of .* > or > Using explicit .svg extension instead of .* > > > Ok
[dpdk-dev] [PATCH v3] devtools: add explicit warning messages for forbidden tokens
Replace the content of warning in the forbidden tokens script
from using the searched regex into using explicit messages
Signed-off-by: Arnon Warshavsky
---
v2 - fix typo
v3 - reduce message not to exceed 80 chars
devtools/check-forbidden-tokens.awk | 7 +--
devtools/checkpatches.sh| 2 ++
2 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/devtools/check-forbidden-tokens.awk
b/devtools/check-forbidden-tokens.awk
index fd77cdd..8c89de3 100755
--- a/devtools/check-forbidden-tokens.awk
+++ b/devtools/check-forbidden-tokens.awk
@@ -63,12 +63,7 @@ BEGIN {
END {
if (count > 0) {
print "Warning in " substr(last_file,6) ":"
- print "are you sure you want to add the following:"
- for (key in expressions) {
- if (expressions[key] > 0) {
- print key
- }
- }
+ print MESSAGE
exit RET_ON_FAIL
}
}
diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
index fb9e9f7..7f823e0 100755
--- a/devtools/checkpatches.sh
+++ b/devtools/checkpatches.sh
@@ -49,6 +49,7 @@ check_forbidden_additions() { #
awk -v FOLDERS="lib drivers" \
-v EXPRESSIONS="rte_panic\\\( rte_exit\\\(" \
-v RET_ON_FAIL=1 \
+ -v MESSAGE='Using rte_panic/rte_exit' \
-f $(dirname $(readlink -e $0))/check-forbidden-tokens.awk \
"$1"
# svg figures must be included with wildcard extension
@@ -56,6 +57,7 @@ check_forbidden_additions() { #
awk -v FOLDERS='doc' \
-v EXPRESSIONS='::[[:space:]]*[^[:space:]]*\\.svg' \
-v RET_ON_FAIL=1 \
+ -v MESSAGE="Using explicit .svg extension instead of .*" \
-f $(dirname $(readlink -e $0))/check-forbidden-tokens.awk \
"$1"
}
--
1.8.3.1
[dpdk-dev] [PATCH v11] devtools: alert on new instances of rte_panic and rte_exit
This patch adds a new function that is called
per every checked patch,
and alerts for new instances of rte_panic/rte_exit.
The check excludes comments, and alerts in the case
of a positive balance between additions and removals.
Signed-off-by: Arnon Warshavsky
Reviewed-by: Stephen Hemminger
---
v11
- Use Neils tmpfile infrastructure to consume the patches
- Allow different calls to the awk script decide if they want
to fail the check or just warn
devtools/checkpatches.sh | 95
1 file changed, 95 insertions(+)
diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
index 1439bce..22832ef 100755
--- a/devtools/checkpatches.sh
+++ b/devtools/checkpatches.sh
@@ -43,6 +43,94 @@ print_usage () {
END_OF_HELP
}
+
+check_forbidden_additions() { #
+# -
+#This awk script receives a list of expressions to monitor
+#and a list of folders to search these expressions in
+# - No search is done inside comments
+# - Both additions and removals of the expressions are checked
+# A positive balance of additions fails the check
+ read -d '' awk_script << 'EOF'
+ BEGIN{
+ split(FOLDERS,deny_folders," ");
+ split(EXPRESSIONS,deny_expr," ");
+ in_file=0;
+ in_comment=0;
+ count=0;
+ comment_start="/*"
+ comment_end="*/"
+ }
+ # search for add/remove instances in current file
+ # state machine assumes the comments structure is enforced by
+ # checkpatches.pl
+ (in_file) {
+ # comment start
+ if (index($0,comment_start) > 0){
+ in_comment = 1
+ }
+ # non comment code
+ if (in_comment == 0) {
+ for (i in deny_expr) {
+ forbidden_added = "^\+.*" deny_expr[i];
+ forbidden_removed="^-.*" deny_expr[i];
+ current = expressions[deny_expr[i]]
+ if ($0 ~ forbidden_added) {
+ count = count + 1;
+ expressions[deny_expr[i]] = current + 1
+ }
+ if ($0 ~ forbidden_removed) {
+ count = count - 1;
+ expressions[deny_expr[i]] = current - 1
+ }
+ }
+ }
+
+ # comment end
+ if (index($0,comment_end) > 0) {
+ in_comment = 0
+ }
+ }
+ # switch to next file , check if the balance of add/remove
+ # of previous filehad new additions
+ ($0 ~ "^\+\+\+ b/") {
+ in_file = 0;
+ if (count > 0){
+ exit;
+ }
+ for (i in deny_folders){
+ re = "^\+\+\+ b/" deny_folders[i];
+ if ($0 ~ deny_folders[i]) {
+ in_file = 1
+ last_file = $0
+ }
+ }
+ }
+ END{
+ if (count > 0){
+ warnText = "\\033[1;31m" "Warning:" "\\033[0m"
+ print warnText " in " substr(last_file,6) ":"
+ print "are you sure you want to add the following:"
+ for (key in expressions) {
+ if (expressions[key] > 0) {
+ print key
+ }
+ }
+ exit RET_ON_FAIL
+ }
+ }
+EOF
+ # -
+
+ # refrain from new additions of rte_panic() and rte_exit()
+ # under lib and net
+ # multiple folders and expressions are separated by spaces
+ awk -v FOLDERS="lib net" \
+ -v EXPRESSIONS="rte_panic\\\( rte_exit\\\(" \
+ -v RET_ON_FAIL=0 \
+ "$awk_script" -
+}
+
number=0
quiet=false
verbose=false
@@ -97,6 +185,13 @@ check () { #
ret=1
fi
+ ! $verbose || printf '\nChecking forbidden tokens additions/removals:\n'
+ report=$(cat $tmpinput | check_forbidden_additions)
+ if [ $? -ne 0 ] ; then
+ ret=1
+ fi
+ printf '%s\n' "$report"
+
clean_tmp_files
[ $ret -eq 0 ] && return 0
--
1.8.3.1
[dpdk-dev] [PATCH v12] devtools: alert on new instances of rte_panic and rte_exit
This patch adds a new function that is called
per every checked patch,
and alerts for new instances of rte_panic/rte_exit.
The check excludes comments, and alerts in the case
of a positive balance between additions and removals.
Signed-off-by: Arnon Warshavsky
Reviewed-by: Stephen Hemminger
Tested-by: Kevin Traynor
---
v11
- Use Neils tmpfile infrastructure to consume the patches
- Allow different calls to the awk script decide if they want
to fail the check or just warn
v12
- Restored Tested message
devtools/checkpatches.sh | 95
1 file changed, 95 insertions(+)
diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
index 1439bce..22832ef 100755
--- a/devtools/checkpatches.sh
+++ b/devtools/checkpatches.sh
@@ -43,6 +43,94 @@ print_usage () {
END_OF_HELP
}
+
+check_forbidden_additions() { #
+# -
+#This awk script receives a list of expressions to monitor
+#and a list of folders to search these expressions in
+# - No search is done inside comments
+# - Both additions and removals of the expressions are checked
+# A positive balance of additions fails the check
+ read -d '' awk_script << 'EOF'
+ BEGIN{
+ split(FOLDERS,deny_folders," ");
+ split(EXPRESSIONS,deny_expr," ");
+ in_file=0;
+ in_comment=0;
+ count=0;
+ comment_start="/*"
+ comment_end="*/"
+ }
+ # search for add/remove instances in current file
+ # state machine assumes the comments structure is enforced by
+ # checkpatches.pl
+ (in_file) {
+ # comment start
+ if (index($0,comment_start) > 0){
+ in_comment = 1
+ }
+ # non comment code
+ if (in_comment == 0) {
+ for (i in deny_expr) {
+ forbidden_added = "^\+.*" deny_expr[i];
+ forbidden_removed="^-.*" deny_expr[i];
+ current = expressions[deny_expr[i]]
+ if ($0 ~ forbidden_added) {
+ count = count + 1;
+ expressions[deny_expr[i]] = current + 1
+ }
+ if ($0 ~ forbidden_removed) {
+ count = count - 1;
+ expressions[deny_expr[i]] = current - 1
+ }
+ }
+ }
+
+ # comment end
+ if (index($0,comment_end) > 0) {
+ in_comment = 0
+ }
+ }
+ # switch to next file , check if the balance of add/remove
+ # of previous filehad new additions
+ ($0 ~ "^\+\+\+ b/") {
+ in_file = 0;
+ if (count > 0){
+ exit;
+ }
+ for (i in deny_folders){
+ re = "^\+\+\+ b/" deny_folders[i];
+ if ($0 ~ deny_folders[i]) {
+ in_file = 1
+ last_file = $0
+ }
+ }
+ }
+ END{
+ if (count > 0){
+ warnText = "\\033[1;31m" "Warning:" "\\033[0m"
+ print warnText " in " substr(last_file,6) ":"
+ print "are you sure you want to add the following:"
+ for (key in expressions) {
+ if (expressions[key] > 0) {
+ print key
+ }
+ }
+ exit RET_ON_FAIL
+ }
+ }
+EOF
+ # -
+
+ # refrain from new additions of rte_panic() and rte_exit()
+ # under lib and net
+ # multiple folders and expressions are separated by spaces
+ awk -v FOLDERS="lib net" \
+ -v EXPRESSIONS="rte_panic\\\( rte_exit\\\(" \
+ -v RET_ON_FAIL=0 \
+ "$awk_script" -
+}
+
number=0
quiet=false
verbose=false
@@ -97,6 +185,13 @@ check () { #
ret=1
fi
+ ! $verbose || printf '\nChecking forbidden tokens additions/removals:\n'
+ report=$(cat $tmpinput | check_forbidden_additions)
+ if [ $? -ne 0 ] ; then
+ ret=1
+ fi
+ printf '%s\n' "$report"
+
clean_tmp_files
[ $ret -eq 0 ] && return 0
--
1.8.3.1
Re: [dpdk-dev] [PATCH v12] devtools: alert on new instances of rte_panic and rte_exit
> > +check_forbidden_additions() { #
>
> This function looks to work with stdin, not a file.
> Better to remove the comment about a .
>
It can actually work with both but you are right. The comment is not
beneficial there
Will fix with the rest of the list below
...
>
> > + if [ $? -ne 0 ] ; then
> > + ret=1
> > + fi
> > + printf '%s\n' "$report"
>
> You are printing the report, no matter of the result? Why?
> Is it because a warning does not return as an error?
>
There is maybe an improvement required here.
>
Yes this is indeed the reason.
Sticking to a simple success/fail in the return value with the price of
printing an empty string in the case of no-warning seemed better than
handling multiple (>2) return codes .
Do you have a preferred way here?
Re: [dpdk-dev] [PATCH v12] devtools: alert on new instances of rte_panic and rte_exit
> > + read -d '' awk_script << 'EOF' > > EOF must be quoted? > Missed that one. Yes, the quotes are there to prevent parameter expansion of the awk variables. The script breaks without them
Re: [dpdk-dev] [PATCH v12] devtools: alert on new instances of rte_panic and rte_exit
> > One more nit: the title says "additions/removals" but the function is > only about additions. > > > Yup. No removals there..
[dpdk-dev] [PATCH v13] devtools: alert on new instances of rte_panic and rte_exit
This patch adds a new function that is called
per every checked patch,
and alerts for new instances of rte_panic/rte_exit.
The check excludes comments, and alerts in the case
of a positive balance between additions and removals.
Signed-off-by: Arnon Warshavsky
Reviewed-by: Stephen Hemminger
Tested-by: Kevin Traynor
---
v11
- Use Neils tmpfile infrastructure to consume the patches
- Allow different calls to the awk script decide if they want
to fail the check or just warn
v12
- Restored Tested message
V13
- Multiple formatting comments by Thomas
- Refrain from printing blank success string
devtools/checkpatches.sh | 92
1 file changed, 92 insertions(+)
diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
index e97a4f2..e8e0bab 100755
--- a/devtools/checkpatches.sh
+++ b/devtools/checkpatches.sh
@@ -43,6 +43,91 @@ print_usage () {
END_OF_HELP
}
+
+check_forbidden_additions() {
+# This awk script receives a list of expressions to monitor
+# and a list of folders to search these expressions in
+# - No search is done inside comments
+# - Both additions and removals of the expressions are checked
+# A positive balance of additions fails the check
+ read -d '' awk_script << 'EOF'
+ BEGIN {
+ split(FOLDERS,deny_folders," ");
+ split(EXPRESSIONS,deny_expr," ");
+ in_file=0;
+ in_comment=0;
+ count=0;
+ comment_start="/*"
+ comment_end="*/"
+ }
+ # search for add/remove instances in current file
+ # state machine assumes the comments structure is enforced by
+ # checkpatches.pl
+ (in_file) {
+ # comment start
+ if (index($0,comment_start) > 0){
+ in_comment = 1
+ }
+ # non comment code
+ if (in_comment == 0) {
+ for (i in deny_expr) {
+ forbidden_added = "^\+.*" deny_expr[i];
+ forbidden_removed="^-.*" deny_expr[i];
+ current = expressions[deny_expr[i]]
+ if ($0 ~ forbidden_added) {
+ count = count + 1;
+ expressions[deny_expr[i]] = current + 1
+ }
+ if ($0 ~ forbidden_removed) {
+ count = count - 1;
+ expressions[deny_expr[i]] = current - 1
+ }
+ }
+ }
+ # comment end
+ if (index($0,comment_end) > 0) {
+ in_comment = 0
+ }
+ }
+ # switch to next file , check if the balance of add/remove
+ # of previous filehad new additions
+ ($0 ~ "^\+\+\+ b/") {
+ in_file = 0;
+ if (count > 0){
+ exit;
+ }
+ for (i in deny_folders){
+ re = "^\+\+\+ b/" deny_folders[i];
+ if ($0 ~ deny_folders[i]) {
+ in_file = 1
+ last_file = $0
+ }
+ }
+ }
+ END{
+ if (count > 0){
+ print "Warning in " substr(last_file,6) ":"
+ print "are you sure you want to add the following:"
+ for (key in expressions) {
+ if (expressions[key] > 0) {
+ print key
+ }
+ }
+ exit RET_ON_FAIL
+ }
+ }
+EOF
+ # -
+
+ # refrain from new additions of rte_panic() and rte_exit()
+ # under lib and net
+ # multiple folders and expressions are separated by spaces
+ awk -v FOLDERS="lib net drivers" \
+ -v EXPRESSIONS="rte_panic\\\( rte_exit\\\(" \
+ -v RET_ON_FAIL=1 \
+ "$awk_script" -
+}
+
number=0
quiet=false
verbose=false
@@ -97,6 +182,13 @@ check () { #
ret=1
fi
+ ! $verbose || printf '\nChecking forbidden tokens additions:\n'
+ report=$(check_forbidden_additions <"$tmpinput")
+ if [ $? -ne 0 ] ; then
+ printf '%s\n' "$report"
+ ret=1
+ fi
+
clean_tmp_files
[ $ret -eq 0 ] && return 0
--
1.8.3.1
Re: [dpdk-dev] [PATCH v13] devtools: alert on new instances of rte_panic and rte_exit
> + awk -v FOLDERS="lib net drivers" \ > > I don't know why net is listed as a root directory here? > I am going to apply the patch without the "net" directory. OK? > Yes please. Its not necessarily root rather in any location of the path, but having added drivers, net is now redundant. thanks /Arnon
Re: [dpdk-dev] [PATCH] devtools: don't use bash extension in checkpatches
Hi Ilya Let's use single quotes instead of variable. Using the script directly with single quotes loses the ability to reuse it with an additional set of folders , expressions and RET_ON_FAIL. If we wish to keep the awk code in this file and not in a separate file, maybe receiving the awk script parameters from the function check_forbidden_additions( ) can also preserve the ability to reuse in future cases. thanks /Arnon
Re: [dpdk-dev] [PATCH] devtools: use bash for checkpatches.sh
On Thu, Aug 30, 2018 at 4:00 PM, Andrzej Ostruszka wrote:
> On some systems /bin/sh does not support '-d' option of 'read'.
> So instead of /bin/sh use /bin/bash.
>
> Fixes: 7413e7f2aeb3 ("devtools: alert on new calls to exit from libs")
> Cc: ar...@qwilt.com
> Signed-off-by: Andrzej Ostruszka
> ---
> devtools/checkpatches.sh | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
> index ba795ad..db7afc4 100755
> --- a/devtools/checkpatches.sh
> +++ b/devtools/checkpatches.sh
> @@ -1,4 +1,4 @@
> -#! /bin/sh
> +#! /bin/bash
> # SPDX-License-Identifier: BSD-3-Clause
> # Copyright 2015 6WIND S.A.
>
> --
> 2.7.4
>
>
Acked-By: Arnon Warshavsky
thanks
/Arnon
[dpdk-dev] [PATCH v4 02/11] bond: replace rte_panic instances in bonding driver
replace panic calls with log and retrun value.
Local functions to this file,
changing from void to int are non-abi-breaking
--
v4 - fix split literal strings in log messages
Signed-off-by: Arnon Warshavsky
---
drivers/net/bonding/rte_eth_bond_8023ad.c | 28 +++
drivers/net/bonding/rte_eth_bond_8023ad_private.h | 2 +-
drivers/net/bonding/rte_eth_bond_api.c| 20 +++-
drivers/net/bonding/rte_eth_bond_pmd.c| 9 +---
drivers/net/bonding/rte_eth_bond_private.h| 2 +-
5 files changed, 40 insertions(+), 21 deletions(-)
diff --git a/drivers/net/bonding/rte_eth_bond_8023ad.c
b/drivers/net/bonding/rte_eth_bond_8023ad.c
index c452318..7512901 100644
--- a/drivers/net/bonding/rte_eth_bond_8023ad.c
+++ b/drivers/net/bonding/rte_eth_bond_8023ad.c
@@ -893,7 +893,7 @@
bond_mode_8023ad_periodic_cb, arg);
}
-void
+int
bond_mode_8023ad_activate_slave(struct rte_eth_dev *bond_dev,
uint16_t slave_id)
{
@@ -939,7 +939,7 @@
timer_cancel(&port->warning_timer);
if (port->mbuf_pool != NULL)
- return;
+ return 0;
RTE_ASSERT(port->rx_ring == NULL);
RTE_ASSERT(port->tx_ring == NULL);
@@ -968,8 +968,9 @@
/* Any memory allocation failure in initialization is critical because
* resources can't be free, so reinitialization is impossible. */
if (port->mbuf_pool == NULL) {
- rte_panic("Slave %u: Failed to create memory pool '%s': %s\n",
- slave_id, mem_name, rte_strerror(rte_errno));
+ RTE_LOG(ERR, PMD, "%s() Slave %u: Failed to create memory pool
'%s': %s\n",
+ __func__, slave_id, mem_name, rte_strerror(rte_errno));
+ return -1;
}
snprintf(mem_name, RTE_DIM(mem_name), "slave_%u_rx", slave_id);
@@ -977,8 +978,9 @@
rte_align32pow2(BOND_MODE_8023AX_SLAVE_RX_PKTS),
socket_id, 0);
if (port->rx_ring == NULL) {
- rte_panic("Slave %u: Failed to create rx ring '%s': %s\n",
slave_id,
- mem_name, rte_strerror(rte_errno));
+ RTE_LOG(ERR, PMD, "%s() Slave %u: Failed to create rx ring
'%s': %s\n",
+ __func__, slave_id, mem_name, rte_strerror(rte_errno));
+ return -1;
}
/* TX ring is at least one pkt longer to make room for marker packet. */
@@ -987,9 +989,12 @@
rte_align32pow2(BOND_MODE_8023AX_SLAVE_TX_PKTS + 1),
socket_id, 0);
if (port->tx_ring == NULL) {
- rte_panic("Slave %u: Failed to create tx ring '%s': %s\n",
slave_id,
- mem_name, rte_strerror(rte_errno));
+ RTE_LOG(ERR, PMD, "%s() Slave %u: Fail to create tx ring '%s':
%s\n",
+ __func__, slave_id, mem_name, rte_strerror(rte_errno));
+ return -1;
}
+
+ return 0;
}
int
@@ -1143,9 +1148,12 @@
struct bond_dev_private *internals = bond_dev->data->dev_private;
uint8_t i;
- for (i = 0; i < internals->active_slave_count; i++)
- bond_mode_8023ad_activate_slave(bond_dev,
+ for (i = 0; i < internals->active_slave_count; i++) {
+ int rc = bond_mode_8023ad_activate_slave(bond_dev,
internals->active_slaves[i]);
+ if (rc != 0)
+ return rc;
+ }
return 0;
}
diff --git a/drivers/net/bonding/rte_eth_bond_8023ad_private.h
b/drivers/net/bonding/rte_eth_bond_8023ad_private.h
index 0f490a5..96a42f2 100644
--- a/drivers/net/bonding/rte_eth_bond_8023ad_private.h
+++ b/drivers/net/bonding/rte_eth_bond_8023ad_private.h
@@ -263,7 +263,7 @@ struct mode8023ad_private {
* @return
* 0 on success, negative value otherwise.
*/
-void
+int
bond_mode_8023ad_activate_slave(struct rte_eth_dev *dev, uint16_t port_id);
/**
diff --git a/drivers/net/bonding/rte_eth_bond_api.c
b/drivers/net/bonding/rte_eth_bond_api.c
index aa89425..96aa1ff 100644
--- a/drivers/net/bonding/rte_eth_bond_api.c
+++ b/drivers/net/bonding/rte_eth_bond_api.c
@@ -69,14 +69,15 @@
return 0;
}
-void
+int
activate_slave(struct rte_eth_dev *eth_dev, uint16_t port_id)
{
struct bond_dev_private *internals = eth_dev->data->dev_private;
uint8_t active_count = internals->active_slave_count;
if (internals->mode == BONDING_MODE_8023AD)
- bond_mode_8023ad_activate_slave(eth_dev, port_id);
+ if (bond_mode_8023ad_activate_slave(eth_dev, port_id) != 0)
+ return -1;
[dpdk-dev] [PATCH v4 03/11] e1000: replace rte_panic instances in e1000 driver
replace panic calls with log and retrun value.
Local function to this file,
changing from void to int is non-abi-breaking
--
v4 - keep error message literal string in a singhle line
Signed-off-by: Arnon Warshavsky
---
drivers/net/e1000/e1000_ethdev.h | 2 +-
drivers/net/e1000/igb_ethdev.c | 3 ++-
drivers/net/e1000/igb_pf.c | 15 +--
3 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/drivers/net/e1000/e1000_ethdev.h b/drivers/net/e1000/e1000_ethdev.h
index 6354b89..2e527de 100644
--- a/drivers/net/e1000/e1000_ethdev.h
+++ b/drivers/net/e1000/e1000_ethdev.h
@@ -411,7 +411,7 @@ int eth_igb_rss_hash_conf_get(struct rte_eth_dev *dev,
/*
* misc function prototypes
*/
-void igb_pf_host_init(struct rte_eth_dev *eth_dev);
+int igb_pf_host_init(struct rte_eth_dev *eth_dev);
void igb_pf_mbx_process(struct rte_eth_dev *eth_dev);
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index 9b808a9..4479616 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -833,7 +833,8 @@ static int igb_flex_filter_uninit(struct rte_eth_dev
*eth_dev)
}
/* initialize PF if max_vfs not zero */
- igb_pf_host_init(eth_dev);
+ if (igb_pf_host_init(eth_dev) != 0)
+ goto err_late;
ctrl_ext = E1000_READ_REG(hw, E1000_CTRL_EXT);
/* Set PF Reset Done bit so PF/VF Mail Ops can work */
diff --git a/drivers/net/e1000/igb_pf.c b/drivers/net/e1000/igb_pf.c
index b9f2e53..ae4b0a4 100644
--- a/drivers/net/e1000/igb_pf.c
+++ b/drivers/net/e1000/igb_pf.c
@@ -63,7 +63,7 @@ int igb_vf_perm_addr_gen(struct rte_eth_dev *dev, uint16_t
vf_num)
return 0;
}
-void igb_pf_host_init(struct rte_eth_dev *eth_dev)
+int igb_pf_host_init(struct rte_eth_dev *eth_dev)
{
struct e1000_vf_info **vfinfo =
E1000_DEV_PRIVATE_TO_P_VFDATA(eth_dev->data->dev_private);
@@ -74,7 +74,7 @@ void igb_pf_host_init(struct rte_eth_dev *eth_dev)
RTE_ETH_DEV_SRIOV(eth_dev).active = 0;
if (0 == (vf_num = dev_num_vf(eth_dev)))
- return;
+ return 0;
if (hw->mac.type == e1000_i350)
nb_queue = 1;
@@ -82,11 +82,14 @@ void igb_pf_host_init(struct rte_eth_dev *eth_dev)
/* per datasheet, it should be 2, but 1 seems correct */
nb_queue = 1;
else
- return;
+ return 0;
*vfinfo = rte_zmalloc("vf_info", sizeof(struct e1000_vf_info) * vf_num,
0);
- if (*vfinfo == NULL)
- rte_panic("Cannot allocate memory for private VF data\n");
+ if (*vfinfo == NULL) {
+ RTE_LOG(CRIT, PMD, "%s(): Cannot allocate memory for private VF
data\n",
+ __func__);
+ return -1;
+ }
RTE_ETH_DEV_SRIOV(eth_dev).active = ETH_8_POOLS;
RTE_ETH_DEV_SRIOV(eth_dev).nb_q_per_pool = nb_queue;
@@ -98,7 +101,7 @@ void igb_pf_host_init(struct rte_eth_dev *eth_dev)
/* set mb interrupt mask */
igb_mb_intr_setup(eth_dev);
- return;
+ return 0;
}
void igb_pf_host_uninit(struct rte_eth_dev *dev)
--
1.8.3.1
[dpdk-dev] [PATCH v4 01/11] crypto: replace rte_panic instances in crypto driver
replace panic calls with log and return value.
--
v2:
- reformat error message to include literal string in a single line
v4: replace -1 return value with -ENOMEM
Signed-off-by: Arnon Warshavsky
---
drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c | 8 +---
drivers/crypto/dpaa_sec/dpaa_sec.c | 8 +---
2 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
b/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
index 23012e3..d465a2d 100644
--- a/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
+++ b/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
@@ -2861,9 +2861,11 @@ struct rte_security_ops dpaa2_sec_security_ops = {
RTE_CACHE_LINE_SIZE,
rte_socket_id());
- if (cryptodev->data->dev_private == NULL)
- rte_panic("Cannot allocate memzone for private "
- "device data");
+ if (cryptodev->data->dev_private == NULL) {
+ RTE_LOG(ERR, PMD, "%s() Cannot allocate memzone for
private device data",
+ __func__);
+ return -ENOMEM;
+ }
}
dpaa2_dev->cryptodev = cryptodev;
diff --git a/drivers/crypto/dpaa_sec/dpaa_sec.c
b/drivers/crypto/dpaa_sec/dpaa_sec.c
index b685220..7b63650 100644
--- a/drivers/crypto/dpaa_sec/dpaa_sec.c
+++ b/drivers/crypto/dpaa_sec/dpaa_sec.c
@@ -2356,9 +2356,11 @@ struct rte_security_ops dpaa_sec_security_ops = {
RTE_CACHE_LINE_SIZE,
rte_socket_id());
- if (cryptodev->data->dev_private == NULL)
- rte_panic("Cannot allocate memzone for private "
- "device data");
+ if (cryptodev->data->dev_private == NULL) {
+ RTE_LOG(ERR, PMD, "%s() Cannot allocate memzone for
private device data",
+ __func__);
+ return -ENOMEM;
+ }
}
dpaa_dev->crypto_dev = cryptodev;
--
1.8.3.1
[dpdk-dev] [PATCH v4 04/11] ixgbe: replace rte_panic instances in ixgbe driver
replace panic calls with log and retrun value.
Local function to this file,
changing from void to int is non-abi-breaking
Signed-off-by: Arnon Warshavsky
---
drivers/net/ixgbe/ixgbe_ethdev.c | 3 ++-
drivers/net/ixgbe/ixgbe_ethdev.h | 2 +-
drivers/net/ixgbe/ixgbe_pf.c | 13 +
3 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index a5e2fc0..c7797f1 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -1224,7 +1224,8 @@ struct rte_ixgbe_xstats_name_off {
memset(hwstrip, 0, sizeof(*hwstrip));
/* initialize PF if max_vfs not zero */
- ixgbe_pf_host_init(eth_dev);
+ if (ixgbe_pf_host_init(eth_dev) != 0)
+ return -1;
ctrl_ext = IXGBE_READ_REG(hw, IXGBE_CTRL_EXT);
/* let hardware know driver is loaded */
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
index 6550777..8bb41ec 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.h
+++ b/drivers/net/ixgbe/ixgbe_ethdev.h
@@ -661,7 +661,7 @@ int ixgbe_fdir_filter_program(struct rte_eth_dev *dev,
void ixgbe_vlan_hw_strip_config(struct rte_eth_dev *dev);
-void ixgbe_pf_host_init(struct rte_eth_dev *eth_dev);
+int ixgbe_pf_host_init(struct rte_eth_dev *eth_dev);
void ixgbe_pf_host_uninit(struct rte_eth_dev *eth_dev);
diff --git a/drivers/net/ixgbe/ixgbe_pf.c b/drivers/net/ixgbe/ixgbe_pf.c
index 4e61310..4cd3651 100644
--- a/drivers/net/ixgbe/ixgbe_pf.c
+++ b/drivers/net/ixgbe/ixgbe_pf.c
@@ -66,7 +66,7 @@ int ixgbe_vf_perm_addr_gen(struct rte_eth_dev *dev, uint16_t
vf_num)
return 0;
}
-void ixgbe_pf_host_init(struct rte_eth_dev *eth_dev)
+int ixgbe_pf_host_init(struct rte_eth_dev *eth_dev)
{
struct ixgbe_vf_info **vfinfo =
IXGBE_DEV_PRIVATE_TO_P_VFDATA(eth_dev->data->dev_private);
@@ -84,11 +84,14 @@ void ixgbe_pf_host_init(struct rte_eth_dev *eth_dev)
RTE_ETH_DEV_SRIOV(eth_dev).active = 0;
vf_num = dev_num_vf(eth_dev);
if (vf_num == 0)
- return;
+ return 0;
*vfinfo = rte_zmalloc("vf_info", sizeof(struct ixgbe_vf_info) * vf_num,
0);
- if (*vfinfo == NULL)
- rte_panic("Cannot allocate memory for private VF data\n");
+ if (*vfinfo == NULL) {
+ RTE_LOG(ERR, PMD, "%s() Cannot allocate memory for private VF
data\n",
+ __func__);
+ return -1;
+ }
memset(mirror_info, 0, sizeof(struct ixgbe_mirror_info));
memset(uta_info, 0, sizeof(struct ixgbe_uta_info));
@@ -116,6 +119,8 @@ void ixgbe_pf_host_init(struct rte_eth_dev *eth_dev)
/* set mb interrupt mask */
ixgbe_mb_intr_setup(eth_dev);
+
+ return 0;
}
void ixgbe_pf_host_uninit(struct rte_eth_dev *eth_dev)
--
1.8.3.1
[dpdk-dev] [PATCH v4 00/11] eal: replace calls to rte_panic and refrain from new instances
The purpose of this patch series is to cleanup the library code from paths that end up aborting the process, and move to checking error values, in order to allow the running process perform an orderly teardown or other mitigation of the event. This patch modifies the majority of rte_panic calls under lib and drivers, and replaces them with a log message and an error return code according to context, that can be propagated up the call stack. - Focus was given to the dpdk initialization path - Some of the panic calls within drivers were left in place where the call is from within an interrupt or calls that are on the data path,where there is no simple applicative route to propagate the error to temination. These should be handled by the driver maintainers. - In order to avoid breaking ABI where panic was called from public void functions, a panic state variable was introduced so that it can be queried after calling these void functions. This tool place for a single function call. - local void functions with no api were changed to retrun a value where needed - No change took place in example and test files - No change took place for debug assertions calling panic - A new function was added to devtools/checkpatches.sh in order to prevent new additions of calls to rte_panic under lib and drivers. Keep calm and don't panic --- v2: - reformat error messages so that literal string are in the same line - fix typo in commit message - add new return code to doxigen of rte_memzone_free() v3: - submit all 13 patches changed and unchanged in the same patchset v4: - remove 2 patches that are no more relevant - fix split literal string in error message - change return value -1 to enum - split value and success code in a static function Arnon Warshavsky (11): crypto: replace rte_panic instances in crypto driver bond: replace rte_panic instances in bonding driver e1000: replace rte_panic instances in e1000 driver ixgbe: replace rte_panic instances in ixgbe driver eal: replace rte_panic instances in eventdev kni: replace rte_panic instances in kni eal: replace rte_panic instances in hugepage_info eal: replace rte_panic instances in interrupts thread eal: replace rte_panic instances in ethdev eal: replace rte_panic instances in init sequence devtools: prevent new instances of rte_panic and rte_exit devtools/checkpatches.sh | 94 - drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c | 8 +- drivers/crypto/dpaa_sec/dpaa_sec.c| 8 +- drivers/net/bonding/rte_eth_bond_8023ad.c | 28 +++-- drivers/net/bonding/rte_eth_bond_8023ad_private.h | 2 +- drivers/net/bonding/rte_eth_bond_api.c| 20 ++-- drivers/net/bonding/rte_eth_bond_pmd.c| 9 +- drivers/net/bonding/rte_eth_bond_private.h| 2 +- drivers/net/e1000/e1000_ethdev.h | 2 +- drivers/net/e1000/igb_ethdev.c| 3 +- drivers/net/e1000/igb_pf.c| 15 +-- drivers/net/ixgbe/ixgbe_ethdev.c | 3 +- drivers/net/ixgbe/ixgbe_ethdev.h | 2 +- drivers/net/ixgbe/ixgbe_pf.c | 13 ++- lib/librte_eal/bsdapp/eal/eal.c | 86 +++- lib/librte_eal/bsdapp/eal/eal_thread.c| 65 +--- lib/librte_eal/common/eal_common_launch.c | 21 lib/librte_eal/common/include/rte_debug.h | 12 +++ lib/librte_eal/linuxapp/eal/eal.c | 120 +++--- lib/librte_eal/linuxapp/eal/eal_hugepage_info.c | 32 -- lib/librte_eal/linuxapp/eal/eal_interrupts.c | 27 +++-- lib/librte_eal/linuxapp/eal/eal_thread.c | 65 +--- lib/librte_ether/rte_ethdev.c | 36 +-- lib/librte_eventdev/rte_eventdev_pmd_pci.h| 8 +- lib/librte_eventdev/rte_eventdev_pmd_vdev.h | 8 +- lib/librte_kni/rte_kni.c | 18 ++-- lib/librte_kni/rte_kni_fifo.h | 11 +- 27 files changed, 533 insertions(+), 185 deletions(-) -- 1.8.3.1
[dpdk-dev] [PATCH v4 05/11] eal: replace rte_panic instances in eventdev
replace panic calls with log and retrun value.
--
v4 - fix split literal strings in log messages
Signed-off-by: Arnon Warshavsky
---
lib/librte_eventdev/rte_eventdev_pmd_pci.h | 8 +---
lib/librte_eventdev/rte_eventdev_pmd_vdev.h | 8 +---
2 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/lib/librte_eventdev/rte_eventdev_pmd_pci.h
b/lib/librte_eventdev/rte_eventdev_pmd_pci.h
index 8fb6138..dad2182 100644
--- a/lib/librte_eventdev/rte_eventdev_pmd_pci.h
+++ b/lib/librte_eventdev/rte_eventdev_pmd_pci.h
@@ -66,9 +66,11 @@
RTE_CACHE_LINE_SIZE,
rte_socket_id());
- if (eventdev->data->dev_private == NULL)
- rte_panic("Cannot allocate memzone for private "
- "device data");
+ if (eventdev->data->dev_private == NULL) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot allocate memzone for
private device data",
+ __func__);
+ return -1;
+ }
}
eventdev->dev = &pci_dev->device;
diff --git a/lib/librte_eventdev/rte_eventdev_pmd_vdev.h
b/lib/librte_eventdev/rte_eventdev_pmd_vdev.h
index 8c64a06..b7c08fa 100644
--- a/lib/librte_eventdev/rte_eventdev_pmd_vdev.h
+++ b/lib/librte_eventdev/rte_eventdev_pmd_vdev.h
@@ -61,9 +61,11 @@
RTE_CACHE_LINE_SIZE,
socket_id);
- if (eventdev->data->dev_private == NULL)
- rte_panic("Cannot allocate memzone for private device"
- " data");
+ if (eventdev->data->dev_private == NULL) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot allocate memzone for
private device data",
+ __func__);
+ return NULL;
+ }
}
return eventdev;
--
1.8.3.1
[dpdk-dev] [PATCH v4 06/11] kni: replace rte_panic instances in kni
replace panic calls with log and retrun value.
Local function to this file,
changing from void to int is non-abi-breaking
Signed-off-by: Arnon Warshavsky
---
lib/librte_kni/rte_kni.c | 18 --
lib/librte_kni/rte_kni_fifo.h | 11 ---
2 files changed, 20 insertions(+), 9 deletions(-)
diff --git a/lib/librte_kni/rte_kni.c b/lib/librte_kni/rte_kni.c
index 8a8f6c1..4dac407 100644
--- a/lib/librte_kni/rte_kni.c
+++ b/lib/librte_kni/rte_kni.c
@@ -353,37 +353,43 @@ struct rte_kni *
/* TX RING */
mz = slot->m_tx_q;
ctx->tx_q = mz->addr;
- kni_fifo_init(ctx->tx_q, KNI_FIFO_COUNT_MAX);
+ if (kni_fifo_init(ctx->tx_q, KNI_FIFO_COUNT_MAX))
+ return NULL;
dev_info.tx_phys = mz->phys_addr;
/* RX RING */
mz = slot->m_rx_q;
ctx->rx_q = mz->addr;
- kni_fifo_init(ctx->rx_q, KNI_FIFO_COUNT_MAX);
+ if (kni_fifo_init(ctx->rx_q, KNI_FIFO_COUNT_MAX))
+ return NULL;
dev_info.rx_phys = mz->phys_addr;
/* ALLOC RING */
mz = slot->m_alloc_q;
ctx->alloc_q = mz->addr;
- kni_fifo_init(ctx->alloc_q, KNI_FIFO_COUNT_MAX);
+ if (kni_fifo_init(ctx->alloc_q, KNI_FIFO_COUNT_MAX))
+ return NULL;
dev_info.alloc_phys = mz->phys_addr;
/* FREE RING */
mz = slot->m_free_q;
ctx->free_q = mz->addr;
- kni_fifo_init(ctx->free_q, KNI_FIFO_COUNT_MAX);
+ if (kni_fifo_init(ctx->free_q, KNI_FIFO_COUNT_MAX))
+ return NULL;
dev_info.free_phys = mz->phys_addr;
/* Request RING */
mz = slot->m_req_q;
ctx->req_q = mz->addr;
- kni_fifo_init(ctx->req_q, KNI_FIFO_COUNT_MAX);
+ if (kni_fifo_init(ctx->req_q, KNI_FIFO_COUNT_MAX))
+ return NULL;
dev_info.req_phys = mz->phys_addr;
/* Response RING */
mz = slot->m_resp_q;
ctx->resp_q = mz->addr;
- kni_fifo_init(ctx->resp_q, KNI_FIFO_COUNT_MAX);
+ if (kni_fifo_init(ctx->resp_q, KNI_FIFO_COUNT_MAX))
+ return NULL;
dev_info.resp_phys = mz->phys_addr;
/* Req/Resp sync mem area */
diff --git a/lib/librte_kni/rte_kni_fifo.h b/lib/librte_kni/rte_kni_fifo.h
index ac26a8c..5052015 100644
--- a/lib/librte_kni/rte_kni_fifo.h
+++ b/lib/librte_kni/rte_kni_fifo.h
@@ -7,17 +7,22 @@
/**
* Initializes the kni fifo structure
*/
-static void
+static int
kni_fifo_init(struct rte_kni_fifo *fifo, unsigned size)
{
/* Ensure size is power of 2 */
- if (size & (size - 1))
- rte_panic("KNI fifo size must be power of 2\n");
+ if (size & (size - 1)) {
+ RTE_LOG(CRIT, EAL, "%s(): KNI fifo size must be power of 2\n",
+ __func__);
+ return -1;
+ }
fifo->write = 0;
fifo->read = 0;
fifo->len = size;
fifo->elem_size = sizeof(void *);
+
+ return 0;
}
/**
--
1.8.3.1
[dpdk-dev] [PATCH v4 07/11] eal: replace rte_panic instances in hugepage_info
replace panic calls with log and retrun value.
v4
static size calculation function changed to return success/fail code
in addition to filling the size result.
Signed-off-by: Arnon Warshavsky
---
lib/librte_eal/linuxapp/eal/eal_hugepage_info.c | 32 -
1 file changed, 21 insertions(+), 11 deletions(-)
diff --git a/lib/librte_eal/linuxapp/eal/eal_hugepage_info.c
b/lib/librte_eal/linuxapp/eal/eal_hugepage_info.c
index fb4b667..debae32 100644
--- a/lib/librte_eal/linuxapp/eal/eal_hugepage_info.c
+++ b/lib/librte_eal/linuxapp/eal/eal_hugepage_info.c
@@ -145,8 +145,8 @@
return num_pages;
}
-static uint64_t
-get_default_hp_size(void)
+static int
+get_default_hp_size(uint64_t *result)
{
const char proc_meminfo[] = "/proc/meminfo";
const char str_hugepagesz[] = "Hugepagesize:";
@@ -155,8 +155,11 @@
unsigned long long size = 0;
FILE *fd = fopen(proc_meminfo, "r");
- if (fd == NULL)
- rte_panic("Cannot open %s\n", proc_meminfo);
+ if (fd == NULL) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot open %s\n",
+ __func__, proc_meminfo);
+ return -1;
+ }
while(fgets(buffer, sizeof(buffer), fd)){
if (strncmp(buffer, str_hugepagesz, hugepagesz_len) == 0){
size = rte_str_to_size(&buffer[hugepagesz_len]);
@@ -164,9 +167,13 @@
}
}
fclose(fd);
- if (size == 0)
- rte_panic("Cannot get default hugepage size from %s\n",
proc_meminfo);
- return size;
+ if (size == 0) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot get default hugepage size from
%s\n",
+__func__, proc_meminfo);
+ return -1;
+ }
+ *result = size;
+ return 0;
}
static const char *
@@ -191,11 +198,14 @@
char *retval = NULL;
FILE *fd = fopen(proc_mounts, "r");
- if (fd == NULL)
- rte_panic("Cannot open %s\n", proc_mounts);
+ if (fd == NULL) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot open %s\n",
+ __func__, proc_mounts);
+ return NULL;
+ }
- if (default_size == 0)
- default_size = get_default_hp_size();
+ if ((default_size == 0) && (get_default_hp_size(&default_size) != 0))
+ return NULL;
while (fgets(buf, sizeof(buf), fd)){
if (rte_strsplit(buf, sizeof(buf), splitstr, _FIELDNAME_MAX,
--
1.8.3.1
[dpdk-dev] [PATCH v4 09/11] eal: replace rte_panic instances in ethdev
Local function to this file,
changing from void to int is non-abi-breaking
Signed-off-by: Arnon Warshavsky
---
lib/librte_ether/rte_ethdev.c | 36 +---
1 file changed, 25 insertions(+), 11 deletions(-)
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 7821a88..9c13827 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -194,7 +194,7 @@ enum {
return port_id;
}
-static void
+static int
rte_eth_dev_shared_data_prepare(void)
{
const unsigned flags = 0;
@@ -210,8 +210,12 @@ enum {
rte_socket_id(), flags);
} else
mz = rte_memzone_lookup(MZ_RTE_ETH_DEV_DATA);
- if (mz == NULL)
- rte_panic("Cannot allocate ethdev shared data\n");
+ if (mz == NULL) {
+ rte_spinlock_unlock(&rte_eth_shared_data_lock);
+ RTE_LOG(CRIT, EAL, "%s(): Cannot allocate ethdev shared
data\n",
+ __func__);
+ return -1;
+ }
rte_eth_dev_shared_data = mz->addr;
if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
@@ -224,6 +228,8 @@ enum {
}
rte_spinlock_unlock(&rte_eth_shared_data_lock);
+
+ return 0;
}
struct rte_eth_dev *
@@ -274,7 +280,8 @@ struct rte_eth_dev *
uint16_t port_id;
struct rte_eth_dev *eth_dev = NULL;
- rte_eth_dev_shared_data_prepare();
+ if (rte_eth_dev_shared_data_prepare() != 0)
+ return NULL;
/* Synchronize port creation between primary and secondary threads. */
rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
@@ -317,7 +324,8 @@ struct rte_eth_dev *
uint16_t i;
struct rte_eth_dev *eth_dev = NULL;
- rte_eth_dev_shared_data_prepare();
+ if (rte_eth_dev_shared_data_prepare() != 0)
+ return NULL;
/* Synchronize port attachment to primary port creation and release. */
rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
@@ -345,7 +353,8 @@ struct rte_eth_dev *
if (eth_dev == NULL)
return -EINVAL;
- rte_eth_dev_shared_data_prepare();
+ if (rte_eth_dev_shared_data_prepare() != 0)
+ return -1;
rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
@@ -399,7 +408,8 @@ struct rte_eth_dev *
int __rte_experimental
rte_eth_dev_owner_new(uint64_t *owner_id)
{
- rte_eth_dev_shared_data_prepare();
+ if (rte_eth_dev_shared_data_prepare() != 0)
+ return -1;
rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
@@ -450,7 +460,8 @@ struct rte_eth_dev *
{
int ret;
- rte_eth_dev_shared_data_prepare();
+ if (rte_eth_dev_shared_data_prepare() != 0)
+ return -1;
rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
@@ -467,7 +478,8 @@ struct rte_eth_dev *
{.id = RTE_ETH_DEV_NO_OWNER, .name = ""};
int ret;
- rte_eth_dev_shared_data_prepare();
+ if (rte_eth_dev_shared_data_prepare() != 0)
+ return -1;
rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
@@ -482,7 +494,8 @@ struct rte_eth_dev *
{
uint16_t port_id;
- rte_eth_dev_shared_data_prepare();
+ if (rte_eth_dev_shared_data_prepare() != 0)
+ return;
rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
@@ -502,7 +515,8 @@ struct rte_eth_dev *
{
int ret = 0;
- rte_eth_dev_shared_data_prepare();
+ if (rte_eth_dev_shared_data_prepare() != 0)
+ return -1;
rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
--
1.8.3.1
[dpdk-dev] [PATCH v4 11/11] devtools: prevent new instances of rte_panic and rte_exit
This patch adds a new function that is called
per every checked patch,
and alerts for new instances of rte_panic/rte_exit.
The check excludes comments, and alerts in the case
of a positive balance between additions and removals.
Signed-off-by: Arnon Warshavsky
---
devtools/checkpatches.sh | 94 +++-
1 file changed, 93 insertions(+), 1 deletion(-)
diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
index 7676a6b..fb37838 100755
--- a/devtools/checkpatches.sh
+++ b/devtools/checkpatches.sh
@@ -61,6 +61,90 @@ print_usage () {
END_OF_HELP
}
+check_forbidden_additions() { #
+# -
+#This awk script receives a list of expressions to monitor
+#and a list of folders to search these expressions in
+# - No search is done inside comments
+# - Both additions and removals of the expressions are checked
+# A positive balance of additions fails the check
+ read -d '' awk_script << 'EOF'
+ BEGIN{
+ split(FOLDERS,deny_folders," ");
+ split(EXPRESSIONS,deny_expr," ");
+ in_file=0;
+ in_comment=0;
+ count=0;
+ comment_start="/*"
+ comment_end="*/"
+}
+# search for add/remove instances in current file
+# state machine assumes the comments structure is enforced by
+# checkpatches.pl
+(in_file) {
+ # comment start
+ if (index($0,comment_start) > 0){
+ in_comment = 1
+ }
+ # non comment code
+ if (in_comment == 0) {
+ for (i in deny_expr) {
+ forbidden_added = "^\+.*" deny_expr[i];
+ forbidden_removed="^-.*" deny_expr[i];
+ current = expressions[deny_expr[i]]
+ if ($0 ~ forbidden_added) {
+ count = count + 1;
+ expressions[deny_expr[i]] = current + 1
+ }
+ if ($0 ~ forbidden_removed) {
+ count = count - 1;
+ expressions[deny_expr[i]] = current - 1
+ }
+ }
+ }
+
+ # comment end
+ if (index($0,comment_end) > 0) {
+ in_comment = 0
+ }
+}
+ # switch to next file , check if the balance of add/remove
+ # of previous filehad new additions
+ ($0 ~ "^\+\+\+ b/") {
+ in_file = 0;
+ if (count > 0){
+ exit;
+ }
+ for (i in deny_folders){
+ re = "^\+\+\+ b/" deny_folders[i];
+ if ($0 ~ deny_folders[i]) {
+ in_file = 1
+ last_file = $0
+ }
+ }
+ }
+ END{
+ if (count > 0){
+ print "Forbidden expressions in " substr(last_file,6)
":"
+ for (key in expressions) {
+ if (expressions[key] > 0) {
+ print key
+ }
+ }
+ exit 1
+ }
+ }
+EOF
+ # -
+
+ # refrain from new additions of rte_panic() and rte_exit()
+ # under lib and net
+ # multiple folders and expressions are separated by spaces
+ awk -v FOLDERS="lib net" \
+ -v EXPRESSIONS="rte_panic\\\( rte_exit\\\(" \
+ "$awk_script" -
+}
+
number=0
quiet=false
verbose=false
@@ -89,11 +173,19 @@ check () { #
total=$(($total + 1))
! $verbose || printf '\n### %s\n\n' "$3"
if [ -n "$1" ] ; then
+ cat "$1" | check_forbidden_additions
+ [ $? -eq 0 ] || return 0
report=$($DPDK_CHECKPATCH_PATH $options "$1" 2>/dev/null)
elif [ -n "$2" ] ; then
- report=$(git format-patch --find-renames --no-stat --stdout -1
$commit |
+ params=$(echo "--find-renames --no-stat --stdout -1")
+ body=$(git format-patch $params $commit)
+ echo "$body" | check_forbidden_additions
+ [ $? -eq 0 ] || return 0
+ report=$(echo "$body" |
$DPDK_CHECKPATCH_PATH $options - 2>/dev/null)
else
+ check_forbidden_additions -
+ [ $? -eq 0 ] || return 0
report=$($DPDK_CHECKPATCH_PATH $options - 2>/dev/null)
fi
[ $? -ne 0 ] || return 0
--
1.8.3.1
[dpdk-dev] [PATCH v4 10/11] eal: replace rte_panic instances in init sequence
Local functions to this file,
changing from void to int are non-abi-breaking.
For handling the single function that cannot
change from void to int due to abi,
where this is the only place it is called in,
I added a state variable that is being checked
right after the call to this function.
--
v4 - fix split literal strings in log messages
Signed-off-by: Arnon Warshavsky
---
lib/librte_eal/bsdapp/eal/eal.c | 86 ++---
lib/librte_eal/bsdapp/eal/eal_thread.c| 65 +++-
lib/librte_eal/common/eal_common_launch.c | 21 ++
lib/librte_eal/common/include/rte_debug.h | 12 +++
lib/librte_eal/linuxapp/eal/eal.c | 120 --
lib/librte_eal/linuxapp/eal/eal_thread.c | 65 +++-
6 files changed, 270 insertions(+), 99 deletions(-)
diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
index d996190..9c2f6f1 100644
--- a/lib/librte_eal/bsdapp/eal/eal.c
+++ b/lib/librte_eal/bsdapp/eal/eal.c
@@ -151,7 +151,7 @@ enum rte_iova_mode
* We also don't lock the whole file, so that in future we can use read-locks
* on other parts, e.g. memzones, to detect if there are running secondary
* processes. */
-static void
+static int
rte_eal_config_create(void)
{
void *rte_mem_cfg_addr;
@@ -160,60 +160,78 @@ enum rte_iova_mode
const char *pathname = eal_runtime_config_path();
if (internal_config.no_shconf)
- return;
+ return 0;
if (mem_cfg_fd < 0){
mem_cfg_fd = open(pathname, O_RDWR | O_CREAT, 0660);
- if (mem_cfg_fd < 0)
- rte_panic("Cannot open '%s' for rte_mem_config\n",
pathname);
+ if (mem_cfg_fd < 0) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot open '%s' for
rte_mem_config\n",
+ __func__, pathname);
+ return -1;
+ }
}
retval = ftruncate(mem_cfg_fd, sizeof(*rte_config.mem_config));
if (retval < 0){
close(mem_cfg_fd);
- rte_panic("Cannot resize '%s' for rte_mem_config\n", pathname);
+ RTE_LOG(CRIT, EAL, "%s(): Cannot resize '%s' for
rte_mem_config\n",
+ __func__, pathname);
+ return -1;
}
retval = fcntl(mem_cfg_fd, F_SETLK, &wr_lock);
if (retval < 0){
close(mem_cfg_fd);
- rte_exit(EXIT_FAILURE, "Cannot create lock on '%s'. Is another
primary "
- "process running?\n", pathname);
+ RTE_LOG(CRIT, EAL, "%s(): Cannot create lock on '%s'. Is
another primary process running?\n",
+ __func__, pathname);
+ return -1;
}
rte_mem_cfg_addr = mmap(NULL, sizeof(*rte_config.mem_config),
PROT_READ | PROT_WRITE, MAP_SHARED, mem_cfg_fd,
0);
if (rte_mem_cfg_addr == MAP_FAILED){
- rte_panic("Cannot mmap memory for rte_config\n");
+ RTE_LOG(CRIT, EAL, "%s(): Cannot mmap memory for rte_config\n",
+ __func__);
+ return -1;
}
memcpy(rte_mem_cfg_addr, &early_mem_config, sizeof(early_mem_config));
rte_config.mem_config = rte_mem_cfg_addr;
+
+ return 0;
}
/* attach to an existing shared memory config */
-static void
+static int
rte_eal_config_attach(void)
{
void *rte_mem_cfg_addr;
const char *pathname = eal_runtime_config_path();
if (internal_config.no_shconf)
- return;
+ return 0;
if (mem_cfg_fd < 0){
mem_cfg_fd = open(pathname, O_RDWR);
- if (mem_cfg_fd < 0)
- rte_panic("Cannot open '%s' for rte_mem_config\n",
pathname);
+ if (mem_cfg_fd < 0) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot open '%s' for
rte_mem_config\n",
+ __func__, pathname);
+ return -1;
+ }
}
rte_mem_cfg_addr = mmap(NULL, sizeof(*rte_config.mem_config),
PROT_READ | PROT_WRITE, MAP_SHARED, mem_cfg_fd,
0);
close(mem_cfg_fd);
- if (rte_mem_cfg_addr == MAP_FAILED)
- rte_panic("Cannot mmap memory for rte_config\n");
+ if (rte_mem_cfg_addr == MAP_FAILED) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot mmap memory for rte_config\n",
+ __func__);
+ return -1;
+ }
rte_config.mem_config = rte_mem_cfg_addr;
+
+
[dpdk-dev] [PATCH v4 08/11] eal: replace rte_panic instances in interrupts thread
replace panic calls with log and retrun value.
Thread function removes the noretrun attribute.
Signed-off-by: Arnon Warshavsky
---
lib/librte_eal/linuxapp/eal/eal_interrupts.c | 27 ---
1 file changed, 20 insertions(+), 7 deletions(-)
diff --git a/lib/librte_eal/linuxapp/eal/eal_interrupts.c
b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
index 58e9328..8b8650a 100644
--- a/lib/librte_eal/linuxapp/eal/eal_interrupts.c
+++ b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
@@ -785,7 +785,7 @@ struct rte_intr_source {
* @return
* never return;
*/
-static __attribute__((noreturn)) void *
+static void *
eal_intr_thread_main(__rte_unused void *arg)
{
struct epoll_event ev;
@@ -803,8 +803,11 @@ static __attribute__((noreturn)) void *
/* create epoll fd */
int pfd = epoll_create(1);
- if (pfd < 0)
- rte_panic("Cannot create epoll instance\n");
+ if (pfd < 0) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot create epoll
instance\n",
+ __func__);
+ return NULL;
+ }
pipe_event.data.fd = intr_pipe.readfd;
/**
@@ -813,8 +816,11 @@ static __attribute__((noreturn)) void *
*/
if (epoll_ctl(pfd, EPOLL_CTL_ADD, intr_pipe.readfd,
&pipe_event) < 0) {
- rte_panic("Error adding fd to %d epoll_ctl, %s\n",
+ RTE_LOG(CRIT, EAL, "%s(): Error adding fd to %d "
+ "epoll_ctl, %s\n",
+ __func__,
intr_pipe.readfd, strerror(errno));
+ return NULL;
}
numfds++;
@@ -831,9 +837,14 @@ static __attribute__((noreturn)) void *
* into wait list.
*/
if (epoll_ctl(pfd, EPOLL_CTL_ADD,
- src->intr_handle.fd, &ev) < 0){
- rte_panic("Error adding fd %d epoll_ctl, %s\n",
- src->intr_handle.fd, strerror(errno));
+ src->intr_handle.fd, &ev) < 0) {
+ RTE_LOG(CRIT, EAL,
+ "%s(): Error adding fd %d "
+ "epoll_ctl, %s\n",
+ __func__,
+ src->intr_handle.fd,
+ strerror(errno));
+ return NULL;
}
else
numfds++;
@@ -848,6 +859,8 @@ static __attribute__((noreturn)) void *
*/
close(pfd);
}
+
+ return NULL;
}
int
--
1.8.3.1
Re: [dpdk-dev] [PATCH v4 01/11] crypto: replace rte_panic instances in crypto driver
Sure.Will change that in v5
On Thu, Apr 19, 2018 at 1:53 PM, Trahe, Fiona wrote:
> Hi Arnon,
> Can you change subject to crypto/dpaa:... please as it's only affecting
> that driver.
> Fiona
>
> > -Original Message-
> > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Arnon Warshavsky
> > Sent: Thursday, April 19, 2018 7:01 AM
> > To: tho...@monjalon.net; Burakov, Anatoly ;
> Lu, Wenzhuo
> > ; Doherty, Declan ;
> > jerin.ja...@caviumnetworks.com; Richardson, Bruce <
> bruce.richard...@intel.com>; Yigit, Ferruh
> >
> > Cc: dev@dpdk.org; ar...@qwilt.com
> > Subject: [dpdk-dev] [PATCH v4 01/11] crypto: replace rte_panic instances
> in crypto driver
> >
> > replace panic calls with log and return value.
> >
> > --
> > v2:
> > - reformat error message to include literal string in a single line
> > v4: replace -1 return value with -ENOMEM
> >
> > Signed-off-by: Arnon Warshavsky
> > ---
> > drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c | 8 +---
> > drivers/crypto/dpaa_sec/dpaa_sec.c | 8 +---
> > 2 files changed, 10 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
> > b/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
> > index 23012e3..d465a2d 100644
> > --- a/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
> > +++ b/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
> > @@ -2861,9 +2861,11 @@ struct rte_security_ops dpaa2_sec_security_ops = {
> > RTE_CACHE_LINE_SIZE,
> > rte_socket_id());
> >
> > - if (cryptodev->data->dev_private == NULL)
> > - rte_panic("Cannot allocate memzone for private "
> > - "device data");
> > + if (cryptodev->data->dev_private == NULL) {
> > + RTE_LOG(ERR, PMD, "%s() Cannot allocate memzone
> for private device data",
> > + __func__);
> > + return -ENOMEM;
> > + }
> > }
> >
> > dpaa2_dev->cryptodev = cryptodev;
> > diff --git a/drivers/crypto/dpaa_sec/dpaa_sec.c
> b/drivers/crypto/dpaa_sec/dpaa_sec.c
> > index b685220..7b63650 100644
> > --- a/drivers/crypto/dpaa_sec/dpaa_sec.c
> > +++ b/drivers/crypto/dpaa_sec/dpaa_sec.c
> > @@ -2356,9 +2356,11 @@ struct rte_security_ops dpaa_sec_security_ops = {
> > RTE_CACHE_LINE_SIZE,
> > rte_socket_id());
> >
> > - if (cryptodev->data->dev_private == NULL)
> > - rte_panic("Cannot allocate memzone for private "
> > - "device data");
> > + if (cryptodev->data->dev_private == NULL) {
> > + RTE_LOG(ERR, PMD, "%s() Cannot allocate memzone
> for private device data",
> > + __func__);
> > + return -ENOMEM;
> > + }
> > }
> >
> > dpaa_dev->crypto_dev = cryptodev;
> > --
> > 1.8.3.1
>
>
--
*Arnon Warshavsky*
*Qwilt | work: +972-72-2221634 | mobile: +972-50-8583058 | ar...@qwilt.com
*
Re: [dpdk-dev] [PATCH v4 07/11] eal: replace rte_panic instances in hugepage_info
Thanks Anatoly. Will fix that in v5. Is it preferred to keep all version notes in the cover letter alone? On Thu, Apr 19, 2018 at 5:03 PM, Burakov, Anatoly wrote: > On 19-Apr-18 7:01 AM, Arnon Warshavsky wrote: > >> replace panic calls with log and retrun value. >> > > typo: return > > >> v4 >> static size calculation function changed to return success/fail code >> in addition to filling the size result. >> >> Signed-off-by: Arnon Warshavsky >> --- >> > > Please do not add version notes to commit message. You might want to use > "git notes add" for this, or manually edit the patch and add notes after > "---" before sending. > > On patch contents, > > Acked-by: Anatoly Burakov > > -- > Thanks, > Anatoly > -- *Arnon Warshavsky* *Qwilt | work: +972-72-2221634 | mobile: +972-50-8583058 | ar...@qwilt.com *
Re: [dpdk-dev] [PATCH v4 10/11] eal: replace rte_panic instances in init sequence
Copy on the commit message and volatile.
Regarding the new function defunct_and_remain_in_endless_loop ()
I don't think I can put that in a separate patch without breaking the
current patch independence.
On Thu, Apr 19, 2018 at 5:39 PM, Burakov, Anatoly wrote:
> On 19-Apr-18 7:01 AM, Arnon Warshavsky wrote:
>
>> Local functions to this file,
>> changing from void to int are non-abi-breaking.
>> For handling the single function that cannot
>> change from void to int due to abi,
>> where this is the only place it is called in,
>> I added a state variable that is being checked
>> right after the call to this function.
>>
>
> A rewrite of commit message is in order, i think :) Something like this:
>
> Change some functions' return type from void to int. This will not break
> ABI because they are internal only.
>
> (see below for comments on lcore changes)
>
>
>> --
>>
>> v4 - fix split literal strings in log messages
>>
>> Signed-off-by: Arnon Warshavsky
>>
>
> Again, please do not add patch/version notes to the commit message, put
> them after "---". Version history is not for commit messages, it's for
> people reviewing it before merge.
>
> ---
>> lib/librte_eal/bsdapp/eal/eal.c | 86 ++---
>> lib/librte_eal/bsdapp/eal/eal_thread.c| 65 +++-
>> lib/librte_eal/common/eal_common_launch.c | 21 ++
>> lib/librte_eal/common/include/rte_debug.h | 12 +++
>> lib/librte_eal/linuxapp/eal/eal.c | 120
>> --
>> lib/librte_eal/linuxapp/eal/eal_thread.c | 65 +++-
>> 6 files changed, 270 insertions(+), 99 deletions(-)
>>
>> diff --git a/lib/librte_eal/bsdapp/eal/eal.c
>> b/lib/librte_eal/bsdapp/eal/eal.c
>> index d996190..9c2f6f1 100644
>> --- a/lib/librte_eal/bsdapp/eal/eal.c
>> +++ b/lib/librte_eal/bsdapp/eal/eal.c
>> @@ -151,7 +151,7 @@ enum rte_iova_mode
>>* We also don't lock the whole file, so that in future we can use
>> read-locks
>>* on other parts, e.g. memzones, to detect if there are running
>> secondary
>>* processes. */
>> -static void
>> +static int
>>
>
> <...>
>
> +
>> +/* move to panic state and do not return */
>> +static __attribute__((noreturn)) void
>> +defunct_and_remain_in_endless_loop(void)
>> +{
>> + rte_move_to_panic_state();
>> + while (1)
>> + sleep(1);
>> }
>>
>
> It seems like you're mixing two different patchsets here. Maybe it would
> be beneficial to put lcore changes in a separate patch? Technically,
> rte_panic's in lcore are not part of init sequence.
>
> (also, should panic state be volatile?)
>
>
> /* main loop of threads */
>> @@ -106,8 +123,11 @@ void eal_thread_init_master(unsigned lcore_id)
>> if (thread_id == lcore_config[lcore_id].thread_id)
>> break;
>> }
>> - if (lcore_id == RTE_MAX_LCORE)
>> - rte_panic("cannot retrieve lcore id\n");
>> + if (lcore_id == RTE_MAX_LCORE) {
>> + RTE_LOG(CRIT, EAL, "%s(): Cannot retrieve lcore id\n",
>>
>
>
> --
> Thanks,
> Anatoly
>
--
*Arnon Warshavsky*
*Qwilt | work: +972-72-2221634 | mobile: +972-50-8583058 | ar...@qwilt.com
*
Re: [dpdk-dev] [PATCH v4 07/11] eal: replace rte_panic instances in hugepage_info
Thanks Kevin
Will address that
On Thu, Apr 19, 2018 at 5:36 PM, Kevin Traynor wrote:
> On 04/19/2018 07:01 AM, Arnon Warshavsky wrote:
> > replace panic calls with log and retrun value.
> >
> > v4
> > static size calculation function changed to return success/fail code
> > in addition to filling the size result.
> >
>
> fyi - this patch doesn't apply on master branch without fuzz
>
> > Signed-off-by: Arnon Warshavsky
> > ---
> > lib/librte_eal/linuxapp/eal/eal_hugepage_info.c | 32
> -
> > 1 file changed, 21 insertions(+), 11 deletions(-)
> >
> > diff --git a/lib/librte_eal/linuxapp/eal/eal_hugepage_info.c
> b/lib/librte_eal/linuxapp/eal/eal_hugepage_info.c
> > index fb4b667..debae32 100644
> > --- a/lib/librte_eal/linuxapp/eal/eal_hugepage_info.c
> > +++ b/lib/librte_eal/linuxapp/eal/eal_hugepage_info.c
> > @@ -145,8 +145,8 @@
> > return num_pages;
> > }
> >
> > -static uint64_t
> > -get_default_hp_size(void)
> > +static int
> > +get_default_hp_size(uint64_t *result)
> > {
> > const char proc_meminfo[] = "/proc/meminfo";
> > const char str_hugepagesz[] = "Hugepagesize:";
> > @@ -155,8 +155,11 @@
> > unsigned long long size = 0;
> >
> > FILE *fd = fopen(proc_meminfo, "r");
> > - if (fd == NULL)
> > - rte_panic("Cannot open %s\n", proc_meminfo);
> > + if (fd == NULL) {
> > + RTE_LOG(CRIT, EAL, "%s(): Cannot open %s\n",
> > + __func__, proc_meminfo);
> > + return -1;
> > + }
> > while(fgets(buffer, sizeof(buffer), fd)){
> > if (strncmp(buffer, str_hugepagesz, hugepagesz_len) == 0){
> > size = rte_str_to_size(&buffer[hugepagesz_len]);
> > @@ -164,9 +167,13 @@
> > }
> > }
> > fclose(fd);
> > - if (size == 0)
> > - rte_panic("Cannot get default hugepage size from %s\n",
> proc_meminfo);
> > - return size;
> > + if (size == 0) {
> > + RTE_LOG(CRIT, EAL, "%s(): Cannot get default hugepage size
> from %s\n",
> > + __func__, proc_meminfo);
> > + return -1;
> > + }
> > + *result = size;
> > + return 0;
> > }
> >
> > static const char *
> > @@ -191,11 +198,14 @@
> > char *retval = NULL;
> >
> > FILE *fd = fopen(proc_mounts, "r");
> > - if (fd == NULL)
> > - rte_panic("Cannot open %s\n", proc_mounts);
> > + if (fd == NULL) {
> > + RTE_LOG(CRIT, EAL, "%s(): Cannot open %s\n",
> > + __func__, proc_mounts);
> > + return NULL;
> > + }
> >
> > - if (default_size == 0)
> > - default_size = get_default_hp_size();
> > + if ((default_size == 0) && (get_default_hp_size(&default_size) !=
> 0))
> > + return NULL;
> >
> > while (fgets(buf, sizeof(buf), fd)){
> > if (rte_strsplit(buf, sizeof(buf), splitstr,
> _FIELDNAME_MAX,
> >
>
>
--
*Arnon Warshavsky*
*Qwilt | work: +972-72-2221634 | mobile: +972-50-8583058 | ar...@qwilt.com
*
Re: [dpdk-dev] [PATCH v4 02/11] bond: replace rte_panic instances in bonding driver
Will do. Thanks
On Thu, Apr 19, 2018 at 8:25 PM, Kevin Traynor wrote:
> On 04/19/2018 07:01 AM, Arnon Warshavsky wrote:
> > replace panic calls with log and retrun value.
> > Local functions to this file,
> > changing from void to int are non-abi-breaking
> > --
> > v4 - fix split literal strings in log messages
> >
> > Signed-off-by: Arnon Warshavsky
> > ---
> > drivers/net/bonding/rte_eth_bond_8023ad.c | 28
> +++
> > drivers/net/bonding/rte_eth_bond_8023ad_private.h | 2 +-
> > drivers/net/bonding/rte_eth_bond_api.c| 20 +++-
> > drivers/net/bonding/rte_eth_bond_pmd.c| 9 +---
> > drivers/net/bonding/rte_eth_bond_private.h| 2 +-
> > 5 files changed, 40 insertions(+), 21 deletions(-)
> >
> > diff --git a/drivers/net/bonding/rte_eth_bond_8023ad.c
> b/drivers/net/bonding/rte_eth_bond_8023ad.c
> > index c452318..7512901 100644
> > --- a/drivers/net/bonding/rte_eth_bond_8023ad.c
> > +++ b/drivers/net/bonding/rte_eth_bond_8023ad.c
> > @@ -893,7 +893,7 @@
> > bond_mode_8023ad_periodic_cb, arg);
> > }
> >
> > -void
> > +int
> > bond_mode_8023ad_activate_slave(struct rte_eth_dev *bond_dev,
> > uint16_t slave_id)
> > {
> > @@ -939,7 +939,7 @@
> > timer_cancel(&port->warning_timer);
> >
> > if (port->mbuf_pool != NULL)
> > - return;
> > + return 0;
> >
> > RTE_ASSERT(port->rx_ring == NULL);
> > RTE_ASSERT(port->tx_ring == NULL);
> > @@ -968,8 +968,9 @@
> > /* Any memory allocation failure in initialization is critical
> because
> >* resources can't be free, so reinitialization is impossible. */
> > if (port->mbuf_pool == NULL) {
> > - rte_panic("Slave %u: Failed to create memory pool '%s':
> %s\n",
> > - slave_id, mem_name, rte_strerror(rte_errno));
> > + RTE_LOG(ERR, PMD, "%s() Slave %u: Failed to create memory
> pool '%s': %s\n",
> > + __func__, slave_id, mem_name,
> rte_strerror(rte_errno));
> > + return -1;
> > }
> >
> > snprintf(mem_name, RTE_DIM(mem_name), "slave_%u_rx", slave_id);
> > @@ -977,8 +978,9 @@
> > rte_align32pow2(BOND_MODE_8023AX_SLAVE_RX_PKTS),
> socket_id, 0);
> >
> > if (port->rx_ring == NULL) {
> > - rte_panic("Slave %u: Failed to create rx ring '%s': %s\n",
> slave_id,
> > - mem_name, rte_strerror(rte_errno));
> > + RTE_LOG(ERR, PMD, "%s() Slave %u: Failed to create rx ring
> '%s': %s\n",
> > + __func__, slave_id, mem_name,
> rte_strerror(rte_errno));
> > + return -1;
> > }
> >
> > /* TX ring is at least one pkt longer to make room for marker
> packet. */
> > @@ -987,9 +989,12 @@
> > rte_align32pow2(BOND_MODE_8023AX_SLAVE_TX_PKTS +
> 1), socket_id, 0);
> >
> > if (port->tx_ring == NULL) {
> > - rte_panic("Slave %u: Failed to create tx ring '%s': %s\n",
> slave_id,
> > - mem_name, rte_strerror(rte_errno));
> > + RTE_LOG(ERR, PMD, "%s() Slave %u: Fail to create tx ring
> '%s': %s\n",
> > + __func__, slave_id, mem_name,
> rte_strerror(rte_errno));
> > + return -1;
> > }
> > +
> > + return 0;
> > }
> >
> > int
> > @@ -1143,9 +1148,12 @@
> > struct bond_dev_private *internals = bond_dev->data->dev_private;
> > uint8_t i;
> >
> > - for (i = 0; i < internals->active_slave_count; i++)
> > - bond_mode_8023ad_activate_slave(bond_dev,
> > + for (i = 0; i < internals->active_slave_count; i++) {
> > + int rc = bond_mode_8023ad_activate_slave(bond_dev,
> > internals->active_slaves[i]);
> > + if (rc != 0)
> > + return rc;
> > + }
> >
> > return 0;
> > }
> > diff --git a/drivers/net/bonding/rte_eth_bond_8023ad_private.h
> b/drivers/net/bonding/rte_eth_bond_8023ad_private.h
> > index 0f490a5..96a42f2 100644
> > --- a/drivers/net/bonding/rte_eth_bond_8023ad_private.h
> > +++ b/drive
Re: [dpdk-dev] [PATCH v4 03/11] e1000: replace rte_panic instances in e1000 driver
Same as in the other patches. Will do. Thanks
On Thu, Apr 19, 2018 at 8:25 PM, Kevin Traynor wrote:
> On 04/19/2018 07:01 AM, Arnon Warshavsky wrote:
> > replace panic calls with log and retrun value.
> > Local function to this file,
> > changing from void to int is non-abi-breaking
> > --
> > v4 - keep error message literal string in a singhle line
> >
> > Signed-off-by: Arnon Warshavsky
> > ---
> > drivers/net/e1000/e1000_ethdev.h | 2 +-
> > drivers/net/e1000/igb_ethdev.c | 3 ++-
> > drivers/net/e1000/igb_pf.c | 15 +--
> > 3 files changed, 12 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/net/e1000/e1000_ethdev.h b/drivers/net/e1000/e1000_
> ethdev.h
> > index 6354b89..2e527de 100644
> > --- a/drivers/net/e1000/e1000_ethdev.h
> > +++ b/drivers/net/e1000/e1000_ethdev.h
> > @@ -411,7 +411,7 @@ int eth_igb_rss_hash_conf_get(struct rte_eth_dev
> *dev,
> > /*
> > * misc function prototypes
> > */
> > -void igb_pf_host_init(struct rte_eth_dev *eth_dev);
> > +int igb_pf_host_init(struct rte_eth_dev *eth_dev);
> >
> > void igb_pf_mbx_process(struct rte_eth_dev *eth_dev);
> >
> > diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_
> ethdev.c
> > index 9b808a9..4479616 100644
> > --- a/drivers/net/e1000/igb_ethdev.c
> > +++ b/drivers/net/e1000/igb_ethdev.c
> > @@ -833,7 +833,8 @@ static int igb_flex_filter_uninit(struct rte_eth_dev
> *eth_dev)
> > }
> >
> > /* initialize PF if max_vfs not zero */
> > - igb_pf_host_init(eth_dev);
> > + if (igb_pf_host_init(eth_dev) != 0)
>
> You don't need "!= 0"
>
> You need to set "error" here, or else return it from igb_pf_host_init().
> We know -ENOMEM is the only error that can be returned from
> igb_pf_host_init() but not sure if should assume that.
>
> > + goto err_late;
> >
> > ctrl_ext = E1000_READ_REG(hw, E1000_CTRL_EXT);
> > /* Set PF Reset Done bit so PF/VF Mail Ops can work */
> > diff --git a/drivers/net/e1000/igb_pf.c b/drivers/net/e1000/igb_pf.c
> > index b9f2e53..ae4b0a4 100644
> > --- a/drivers/net/e1000/igb_pf.c
> > +++ b/drivers/net/e1000/igb_pf.c
> > @@ -63,7 +63,7 @@ int igb_vf_perm_addr_gen(struct rte_eth_dev *dev,
> uint16_t vf_num)
> > return 0;
> > }
> >
> > -void igb_pf_host_init(struct rte_eth_dev *eth_dev)
> > +int igb_pf_host_init(struct rte_eth_dev *eth_dev)
> > {
> > struct e1000_vf_info **vfinfo =
> > E1000_DEV_PRIVATE_TO_P_VFDATA(eth_dev->data->dev_private);
> > @@ -74,7 +74,7 @@ void igb_pf_host_init(struct rte_eth_dev *eth_dev)
> >
> > RTE_ETH_DEV_SRIOV(eth_dev).active = 0;
> > if (0 == (vf_num = dev_num_vf(eth_dev)))
> > - return;
> > + return 0;
> >
> > if (hw->mac.type == e1000_i350)
> > nb_queue = 1;
> > @@ -82,11 +82,14 @@ void igb_pf_host_init(struct rte_eth_dev *eth_dev)
> > /* per datasheet, it should be 2, but 1 seems correct */
> > nb_queue = 1;
> > else
> > - return;
> > + return 0;
> >
> > *vfinfo = rte_zmalloc("vf_info", sizeof(struct e1000_vf_info) *
> vf_num, 0);
> > - if (*vfinfo == NULL)
> > - rte_panic("Cannot allocate memory for private VF data\n");
> > + if (*vfinfo == NULL) {
> > + RTE_LOG(CRIT, PMD, "%s(): Cannot allocate memory for
> private VF data\n",
> > + __func__);
> > + return -1;
> > + }
> >
> > RTE_ETH_DEV_SRIOV(eth_dev).active = ETH_8_POOLS;
> > RTE_ETH_DEV_SRIOV(eth_dev).nb_q_per_pool = nb_queue;
> > @@ -98,7 +101,7 @@ void igb_pf_host_init(struct rte_eth_dev *eth_dev)
> > /* set mb interrupt mask */
> > igb_mb_intr_setup(eth_dev);
> >
> > - return;
> > + return 0;
> > }
> >
> > void igb_pf_host_uninit(struct rte_eth_dev *dev)
> >
>
>
--
*Arnon Warshavsky*
*Qwilt | work: +972-72-2221634 | mobile: +972-50-8583058 | ar...@qwilt.com
*
Re: [dpdk-dev] [PATCH v4 07/11] eal: replace rte_panic instances in hugepage_info
Now clear.Thanks On Thu, Apr 19, 2018 at 5:50 PM, Burakov, Anatoly wrote: > On 19-Apr-18 3:45 PM, Burakov, Anatoly wrote: > >> On 19-Apr-18 3:09 PM, Arnon Warshavsky wrote: >> >>> Thanks Anatoly. Will fix that in v5. >>> Is it preferred to keep all version notes in the cover letter alone? >>> >>> >> Generally, cover letter should give general outline (i.e. "fixed 32 bit >> compile"), while notes for individual patches should be more specific about >> the changes between versions (but not too specific, i.e. don't do "change >> variable X on line 100 to be Y"). >> >> So, whatever you think gets your point across best. Not all changes >> deserve to be called out in the cover letter. >> >> Just to be clear: > > With my initial reply, i did not mean "patch notes should be in the cover > letter". What i meant was that you have put your version changes "e.g. v4 - > changed this to that" into the commit message. > > What you should have done is put your patch notes after the commit > message, like this: > > replace panic calls with log and retrun value. > > Signed-off-by: Arnon Warshavsky > --- > > v4 > static size calculation function changed to return success/fail code > in addition to filling the size result. > > > lib/librte_eal/linuxapp/eal/eal_hugepage_info.c | 32 > - > 1 file changed, 21 insertions(+), 11 deletions(-) > > Note that the v4 comments are after the "---" - this is where the commit > message ends as far as git concerned, so you can put your notes there. > > -- > Thanks, > Anatoly > -- *Arnon Warshavsky* *Qwilt | work: +972-72-2221634 | mobile: +972-50-8583058 | ar...@qwilt.com *
Re: [dpdk-dev] [PATCH v4 04/11] ixgbe: replace rte_panic instances in ixgbe driver
> On 04/19/2018 07:01 AM, Arnon Warshavsky wrote: > > replace panic calls with log and retrun value. > > typo return, seems to be in a few commit msgs > Yup. They were not even copy pasted so at least I am consistent :) > + return -1; > > similar comment as previous patch about using an appropriate return value > And those Thanks
Re: [dpdk-dev] [PATCH v4 05/11] eal: replace rte_panic instances in eventdev
Ok. Thanks
On Thu, Apr 19, 2018 at 8:26 PM, Kevin Traynor wrote:
> On 04/19/2018 07:01 AM, Arnon Warshavsky wrote:
> > replace panic calls with log and retrun value.
> >
> > --
> > v4 - fix split literal strings in log messages
> >
> > Signed-off-by: Arnon Warshavsky
> > ---
> > lib/librte_eventdev/rte_eventdev_pmd_pci.h | 8 +---
> > lib/librte_eventdev/rte_eventdev_pmd_vdev.h | 8 +---
> > 2 files changed, 10 insertions(+), 6 deletions(-)
> >
> > diff --git a/lib/librte_eventdev/rte_eventdev_pmd_pci.h
> b/lib/librte_eventdev/rte_eventdev_pmd_pci.h
> > index 8fb6138..dad2182 100644
> > --- a/lib/librte_eventdev/rte_eventdev_pmd_pci.h
> > +++ b/lib/librte_eventdev/rte_eventdev_pmd_pci.h
> > @@ -66,9 +66,11 @@
> > RTE_CACHE_LINE_SIZE,
> > rte_socket_id());
> >
> > - if (eventdev->data->dev_private == NULL)
> > - rte_panic("Cannot allocate memzone for private "
> > - "device data");
> > + if (eventdev->data->dev_private == NULL) {
> > + RTE_LOG(CRIT, EAL, "%s(): Cannot allocate memzone
> for private device data",
> > + __func__);
> > + return -1;
>
> return -ENOMEM
>
> > + }
> > }
> >
> > eventdev->dev = &pci_dev->device;
> > diff --git a/lib/librte_eventdev/rte_eventdev_pmd_vdev.h
> b/lib/librte_eventdev/rte_eventdev_pmd_vdev.h
> > index 8c64a06..b7c08fa 100644
> > --- a/lib/librte_eventdev/rte_eventdev_pmd_vdev.h
> > +++ b/lib/librte_eventdev/rte_eventdev_pmd_vdev.h
> > @@ -61,9 +61,11 @@
> > RTE_CACHE_LINE_SIZE,
> > socket_id);
> >
> > - if (eventdev->data->dev_private == NULL)
> > - rte_panic("Cannot allocate memzone for private
> device"
> > - " data");
> > + if (eventdev->data->dev_private == NULL) {
> > + RTE_LOG(CRIT, EAL, "%s(): Cannot allocate memzone
> for private device data",
> > + __func__);
> > + return NULL;
> > + }
> > }
> >
> > return eventdev;
> >
>
>
--
*Arnon Warshavsky*
*Qwilt | work: +972-72-2221634 | mobile: +972-50-8583058 | ar...@qwilt.com
*
Re: [dpdk-dev] [PATCH v4 08/11] eal: replace rte_panic instances in interrupts thread
Not deliberate.Thanks
On Thu, Apr 19, 2018 at 8:27 PM, Kevin Traynor wrote:
> On 04/19/2018 07:01 AM, Arnon Warshavsky wrote:
> > replace panic calls with log and retrun value.
> > Thread function removes the noretrun attribute.
> >
> > Signed-off-by: Arnon Warshavsky
> > ---
> > lib/librte_eal/linuxapp/eal/eal_interrupts.c | 27
> ---
> > 1 file changed, 20 insertions(+), 7 deletions(-)
> >
> > diff --git a/lib/librte_eal/linuxapp/eal/eal_interrupts.c
> b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
> > index 58e9328..8b8650a 100644
> > --- a/lib/librte_eal/linuxapp/eal/eal_interrupts.c
> > +++ b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
> > @@ -785,7 +785,7 @@ struct rte_intr_source {
> > * @return
> > * never return;
> > */
> > -static __attribute__((noreturn)) void *
> > +static void *
> > eal_intr_thread_main(__rte_unused void *arg)
> > {
> > struct epoll_event ev;
> > @@ -803,8 +803,11 @@ static __attribute__((noreturn)) void *
> >
> > /* create epoll fd */
> > int pfd = epoll_create(1);
> > - if (pfd < 0)
> > - rte_panic("Cannot create epoll instance\n");
> > + if (pfd < 0) {
> > + RTE_LOG(CRIT, EAL, "%s(): Cannot create epoll
> instance\n",
> > + __func__);
> > + return NULL;
> > + }
> >
> > pipe_event.data.fd = intr_pipe.readfd;
> > /**
> > @@ -813,8 +816,11 @@ static __attribute__((noreturn)) void *
> >*/
> > if (epoll_ctl(pfd, EPOLL_CTL_ADD, intr_pipe.readfd,
> > &pipe_event) < 0) {
> > - rte_panic("Error adding fd to %d epoll_ctl, %s\n",
> > + RTE_LOG(CRIT, EAL, "%s(): Error adding fd to %d "
> > + "epoll_ctl, %s\n",
> > + __func__,
> > intr_pipe.readfd, strerror(errno));
> > + return NULL;
> > }
> > numfds++;
> >
> > @@ -831,9 +837,14 @@ static __attribute__((noreturn)) void *
> >* into wait list.
> >*/
> > if (epoll_ctl(pfd, EPOLL_CTL_ADD,
> > - src->intr_handle.fd, &ev) < 0){
> > - rte_panic("Error adding fd %d epoll_ctl,
> %s\n",
> > - src->intr_handle.fd,
> strerror(errno));
> > + src->intr_handle.fd, &ev) < 0) {
>
> The alignment changed here, not sure if it was deliberate
>
> > + RTE_LOG(CRIT, EAL,
> > + "%s(): Error adding fd %d "
> > + "epoll_ctl, %s\n",
> > + __func__,
> > + src->intr_handle.fd,
> > + strerror(errno));
> > + return NULL;
> > }
> > else
> > numfds++;
> > @@ -848,6 +859,8 @@ static __attribute__((noreturn)) void *
> >*/
> > close(pfd);
> > }
> > +
> > + return NULL;
> > }
> >
> > int
> >
>
>
--
*Arnon Warshavsky*
*Qwilt | work: +972-72-2221634 | mobile: +972-50-8583058 | ar...@qwilt.com
*
Re: [dpdk-dev] [PATCH v4 09/11] eal: replace rte_panic instances in ethdev
> > Lots of "!= 0"'s - you might gather by now that I don't like them :-) > No way. Would have never guessed that :) Sure. When in Rome.. > > hmm, I'm wondering should void __rte_experimental > rte_eth_dev_owner_delete change to return an int, now that there is a > fail case and it is still experimental...? > Good point. Missed that thanks /Arnon
Re: [dpdk-dev] [PATCH v4 10/11] eal: replace rte_panic instances in init sequence
On Thu, Apr 19, 2018 at 5:57 PM, Burakov, Anatoly wrote: > On 19-Apr-18 3:48 PM, Arnon Warshavsky wrote: > >> Copy on the commit message and volatile. >> >> Regarding the new function defunct_and_remain_in_endless_loop () >> I don't think I can put that in a separate patch without breaking the >> current patch independence. >> > > How so? > > Just leave some panic instances in there for thread-related stuff and fix > them up in the next patch. > > Also, i'm not sure sending threads into an infinite loop on panic is such > a good idea. You might want to look at Olivier's approach [1] to creating > threads, using pthread_barriers and pthread_kill/cancel. > > This does warrant a separate patch now :) Thanks Anatoly Going into the infinite loop looked like the lesser evil at the time , but I guess I was too eager to get rid of as many panics on this path as I could. Given the bw I will need to properly handle it, and the fact that there are still some more panic calls in the code I will withdraw this specific removal inside the thread and handle it for the next build.
Re: [dpdk-dev] [PATCH v4 10/11] eal: replace rte_panic instances in init sequence
Agree. As I wrote below - I will put this instance back in place for this patchset and handle it on a different one On Thu, Apr 19, 2018 at 8:31 PM, Kevin Traynor wrote: > On 04/19/2018 03:57 PM, Burakov, Anatoly wrote: > > On 19-Apr-18 3:48 PM, Arnon Warshavsky wrote: > >> Copy on the commit message and volatile. > >> > >> Regarding the new function defunct_and_remain_in_endless_loop () > >> I don't think I can put that in a separate patch without breaking the > >> current patch independence. > > > > How so? > > > > Just leave some panic instances in there for thread-related stuff and > > fix them up in the next patch. > > > > Also, i'm not sure sending threads into an infinite loop on panic is > > such a good idea. You might want to look at Olivier's approach [1] to > > creating threads, using pthread_barriers and pthread_kill/cancel. > > > > I haven't reviewed this one yet, but going into an infinite loop doesn't > seem like the right thing to do. > > > This does warrant a separate patch now :) > > > > -- *Arnon Warshavsky* *Qwilt | work: +972-72-2221634 | mobile: +972-50-8583058 | ar...@qwilt.com *
Re: [dpdk-dev] [PATCH v4 10/11] eal: replace rte_panic instances in init sequence
Thanks Aaron
Previously, it wasn't possible for mem_cfg_fd to be reused after a
> failure. Now it is - please reset it to -1. in these close conditions.
>
> Will do.
>
>
> Again, previously this would have aborted on a failure. So it needs to
> be reset to a value that allows retry.
>
Same here
>
> > switch (rte_config.process_type){
> > case RTE_PROC_PRIMARY:
> > - rte_eal_config_create();
> > + if (rte_eal_config_create())
> > + return -1;
> > break;
> > case RTE_PROC_SECONDARY:
> > - rte_eal_config_attach();
> > + if (rte_eal_config_attach())
> > + return -1;
> > rte_eal_mcfg_wait_complete(rte_config.mem_config);
> > break;
> > case RTE_PROC_AUTO:
> > case RTE_PROC_INVALID:
>
> Not for this patch, but I just noticed that this should probably use a
> 'default' case.
>
> Will add this while Im here
>
> Use rte_eal_init_alert to indicate why you are failing the init.
>
Will do
>
> > if (rte_mp_channel_init() < 0) {
> > rte_eal_init_alert("failed to init mp channel\n");
> > @@ -652,7 +676,8 @@ static void rte_eal_init_alert(const char *msg)
> >
> > eal_check_mem_on_local_socket();
> >
> > - eal_thread_init_master(rte_config.master_lcore);
> > + if (eal_thread_init_master(rte_config.master_lcore) != 0)
> > + return -1;
>
> Is it ever possible to recover from this?
Definitely not recoverable, but not different than the other cases where
panic propagate all the way up rather than aborting
> Still needs
> rte_eal_init_alert() call.
>
Will do
>
>
> How are you cleaning up the threads that were spawned? Lets say this
> loop will execute 5 times, and on the 3rd entry, these errors happen.
> You now leave DPDK 'half-initialized' - you've spun up threads and
> allocated memory.
>
...
>
> I don't see how any of this is better for the user. In fact, I think
> this is worse because it will make portions of the application stop
> working without any way to move forward. rte_panic() will at least give
> the process a chance to recover from a potentially ephemeral condition.
>
> As I wrote in a different reply on this patch
I was probably too eager to get rid of this panic taking some wrong
assumptions on the way the library will be called.
Removing the panic from the thread is obviously more complex and also ABI
breaking.
>From my own bw, I will not make it with a proper change to this version, so
I will revert back to panicing on this patchset
and aim for the thread in the next build.
>
> This seems to only exist as a way of triggering the run_once check in
> the eal_init. It doesn't add anything except one more state variable to
> check against. What is the purpose?
>
Actually this is not a run-once in purpose, rather an attempt to define a
state for the device
and on the way work around breaking abi on the the void function called
before that.
> + if (rte_get_panic_state())
> + return -1;
> +
Please just use run_once. That's a better way of preventing this.
>
> As stated above - no a run-once
>
> All of the comments from the bsd side apply here.
>
Re: [dpdk-dev] [PATCH v4 11/11] devtools: prevent new instances of rte_panic and rte_exit
> > I don't think rte_panic should be considered forbidden. Rather their > use should be flagged (as this patch does). However, the 'exit 1' > (which will return a failure for the automatic checkpatch script bot) > might end up problematic as maintainers might consider it a patch that > is not ready. > > I wouldn't object to this patch, but just think you might want to change > the print to something like: > > WARN: Are you sure you meant to use "$expression"? > I can definitely change the warning text , but I think the whole point is actually to prevent future panic calls. If it were a recommendation, one can never converge with the attempt to get rid of them, as not throwing a panic is a lot harder. Thanks Arnon
Re: [dpdk-dev] [PATCH v4 10/11] eal: replace rte_panic instances in init sequence
>
> Looking at the eal_thread_init_master, I think it's probably a
> recoverable condition. For instance, perhaps the core mask was wrong,
> and could be corrected by re-attempting the initialization. Just
> suggesting that it's probably okay to allow a re-attempt here. I would
> suggest:
>
> - eal_thread_init_master(rte_config.master_lcore);
> + if (eal_thread_init_master(rte_config.master_lcore) != 0) {
> + rte_eal_init_alert("Cannot assign master lcore\n");
> + rte_errno = EINVAL;
> + return -1;
> + }
>
> if you agree.
>
> Yes. This is indeed the way to go.
>
> --
>
> In the above I hope it illustrates what you'll need - a way to signal to
> each side that initialization phase is happening, and that
> initialization was successful / failed, and to clean up in the failure
> case.
>
> Just meant for illustration so feel free to ignore / flame, but that's
> how I would go about removing the rte_panic() calls.
>
Thanks.
Indeed I did not consider recovery and clean up from here
>
> > This seems to only exist as a way of triggering the run_once check in
> > the eal_init. It doesn't add anything except one more state variable to
> > check against. What is the purpose?
> >
> >
> > Actually this is not a run-once in purpose, rather an attempt to define
> a state for the device
> > and on the way work around breaking abi on the the void function called
> before that.
>
> I think it's a way to try and track state for initialization and to
> prevent future inits. Which ABI are you worried about? rte_panic()?
> I'm not sure how this is an ABI work around, but I'm probably not
> thinking about it hard enough.
>
I now see the mess I did and why wasn't it clear.
I initially changed the api of eal_thread_init_master() from void to int.
Having had the ABI check failed, I only fixed the linuxapp back to void and
added this workaround to prevent ABI break on this patch.
I will align both linuxapp and bsd to remain at panic for this function as
well , and address it together with the thread itself
--
*Arnon Warshavsky*
*Qwilt | work: +972-72-2221634 | mobile: +972-50-8583058 | ar...@qwilt.com
*
[dpdk-dev] [PATCH v5 00/11] eal: replace calls to rte_panic and refrain from new instances
The purpose of this patch series is to cleanup the library code from paths that end up aborting the process, and move to checking error values, in order to allow the running process perform an orderly teardown or other mitigation of the event. This patch modifies the majority of rte_panic calls under lib and drivers, and replaces them with a log message and an error return code according to context, that can be propagated up the call stack. - Focus was given to the dpdk initialization path - Some of the panic calls within drivers were left in place where the call is from within an interrupt or calls that are on the data path,where there is no simple applicative route to propagate the error to temination. These should be handled by the driver maintainers.. - local void functions with no api were changed to retrun a value where needed - No change took place in example and test files - No change took place for debug assertions calling panic - A new function was added to devtools/checkpatches.sh in order to prevent new additions of calls to rte_panic under lib and drivers. Keep calm and don't panic --- v2: - reformat error messages so that literal string are in the same line - fix typo in commit message - add new return code to doxigen of rte_memzone_free() v3: - submit all 13 patches changed and unchanged in the same patchset v4: - remove 2 patches that are no more relevant - fix split literal string in error message - change return value -1 to enum - split value and success code in a static function v5: - reword commit messages - revert thread related instances back to panicing - handle file descriptors with state to reset after eal init failure in case re initialization takes place Arnon Warshavsky (11): crypto/dpaa: replace rte_panic instances in crypto/dpaa driver bond: replace rte_panic instances in bonding driver e1000: replace rte_panic instances in e1000 driver ixgbe: replace rte_panic instances in ixgbe driver eal: replace rte_panic instances in eventdev kni: replace rte_panic instances in kni eal: replace rte_panic instances in hugepage_info eal: replace rte_panic instances in interrupts thread eal: replace rte_panic instances in ethdev eal: replace rte_panic instances in init sequence devtools: prevent new instances of rte_panic and rte_exit devtools/checkpatches.sh | 95 +- drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c | 8 +- drivers/crypto/dpaa_sec/dpaa_sec.c| 8 +- drivers/net/bonding/rte_eth_bond_8023ad.c | 29 --- drivers/net/bonding/rte_eth_bond_8023ad_private.h | 2 +- drivers/net/bonding/rte_eth_bond_api.c| 22 +++-- drivers/net/bonding/rte_eth_bond_pmd.c| 9 ++- drivers/net/bonding/rte_eth_bond_private.h| 2 +- drivers/net/e1000/e1000_ethdev.h | 2 +- drivers/net/e1000/igb_ethdev.c| 4 +- drivers/net/e1000/igb_pf.c| 15 ++-- drivers/net/ixgbe/ixgbe_ethdev.c | 6 +- drivers/net/ixgbe/ixgbe_ethdev.h | 2 +- drivers/net/ixgbe/ixgbe_pf.c | 15 ++-- lib/librte_eal/bsdapp/eal/eal.c | 70 +++- lib/librte_eal/linuxapp/eal/eal.c | 97 +++ lib/librte_eal/linuxapp/eal/eal_hugepage_info.c | 37 ++--- lib/librte_eal/linuxapp/eal/eal_interrupts.c | 25 -- lib/librte_ether/rte_ethdev.c | 42 +++--- lib/librte_ether/rte_ethdev.h | 4 +- lib/librte_eventdev/rte_eventdev_pmd_pci.h| 8 +- lib/librte_eventdev/rte_eventdev_pmd_vdev.h | 8 +- lib/librte_kni/rte_kni.c | 18 +++-- lib/librte_kni/rte_kni_fifo.h | 11 ++- 24 files changed, 396 insertions(+), 143 deletions(-) -- 1.8.3.1
[dpdk-dev] [PATCH v5 01/11] crypto/dpaa: replace rte_panic instances in crypto/dpaa driver
replace panic calls with log and return value.
Signed-off-by: Arnon Warshavsky
---
drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c | 8 +---
drivers/crypto/dpaa_sec/dpaa_sec.c | 8 +---
2 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
b/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
index 3700d70..cee34f1 100644
--- a/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
+++ b/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
@@ -2864,9 +2864,11 @@ struct rte_security_ops dpaa2_sec_security_ops = {
RTE_CACHE_LINE_SIZE,
rte_socket_id());
- if (cryptodev->data->dev_private == NULL)
- rte_panic("Cannot allocate memzone for private "
- "device data");
+ if (cryptodev->data->dev_private == NULL) {
+ RTE_LOG(ERR, PMD, "%s() Cannot allocate memzone for
private device data",
+ __func__);
+ return -ENOMEM;
+ }
}
dpaa2_dev->cryptodev = cryptodev;
diff --git a/drivers/crypto/dpaa_sec/dpaa_sec.c
b/drivers/crypto/dpaa_sec/dpaa_sec.c
index b685220..7b63650 100644
--- a/drivers/crypto/dpaa_sec/dpaa_sec.c
+++ b/drivers/crypto/dpaa_sec/dpaa_sec.c
@@ -2356,9 +2356,11 @@ struct rte_security_ops dpaa_sec_security_ops = {
RTE_CACHE_LINE_SIZE,
rte_socket_id());
- if (cryptodev->data->dev_private == NULL)
- rte_panic("Cannot allocate memzone for private "
- "device data");
+ if (cryptodev->data->dev_private == NULL) {
+ RTE_LOG(ERR, PMD, "%s() Cannot allocate memzone for
private device data",
+ __func__);
+ return -ENOMEM;
+ }
}
dpaa_dev->crypto_dev = cryptodev;
--
1.8.3.1
[dpdk-dev] [PATCH v5 02/11] bond: replace rte_panic instances in bonding driver
replace panic calls with log and return value.
Local functions to this file,
changing from void to int are non-abi-breaking
Signed-off-by: Arnon Warshavsky
---
drivers/net/bonding/rte_eth_bond_8023ad.c | 29 ++-
drivers/net/bonding/rte_eth_bond_8023ad_private.h | 2 +-
drivers/net/bonding/rte_eth_bond_api.c| 22 -
drivers/net/bonding/rte_eth_bond_pmd.c| 9 ---
drivers/net/bonding/rte_eth_bond_private.h| 2 +-
5 files changed, 42 insertions(+), 22 deletions(-)
diff --git a/drivers/net/bonding/rte_eth_bond_8023ad.c
b/drivers/net/bonding/rte_eth_bond_8023ad.c
index c452318..2fb6cad 100644
--- a/drivers/net/bonding/rte_eth_bond_8023ad.c
+++ b/drivers/net/bonding/rte_eth_bond_8023ad.c
@@ -893,7 +893,7 @@
bond_mode_8023ad_periodic_cb, arg);
}
-void
+int
bond_mode_8023ad_activate_slave(struct rte_eth_dev *bond_dev,
uint16_t slave_id)
{
@@ -939,7 +939,7 @@
timer_cancel(&port->warning_timer);
if (port->mbuf_pool != NULL)
- return;
+ return 0;
RTE_ASSERT(port->rx_ring == NULL);
RTE_ASSERT(port->tx_ring == NULL);
@@ -968,8 +968,9 @@
/* Any memory allocation failure in initialization is critical because
* resources can't be free, so reinitialization is impossible. */
if (port->mbuf_pool == NULL) {
- rte_panic("Slave %u: Failed to create memory pool '%s': %s\n",
- slave_id, mem_name, rte_strerror(rte_errno));
+ RTE_LOG(ERR, PMD, "%s() Slave %u: Failed to create memory pool
'%s': %s\n",
+ __func__, slave_id, mem_name, rte_strerror(rte_errno));
+ return -1;
}
snprintf(mem_name, RTE_DIM(mem_name), "slave_%u_rx", slave_id);
@@ -977,8 +978,9 @@
rte_align32pow2(BOND_MODE_8023AX_SLAVE_RX_PKTS),
socket_id, 0);
if (port->rx_ring == NULL) {
- rte_panic("Slave %u: Failed to create rx ring '%s': %s\n",
slave_id,
- mem_name, rte_strerror(rte_errno));
+ RTE_LOG(ERR, PMD, "%s() Slave %u: Failed to create rx ring
'%s': %s\n",
+ __func__, slave_id, mem_name, rte_strerror(rte_errno));
+ return -1;
}
/* TX ring is at least one pkt longer to make room for marker packet. */
@@ -987,9 +989,12 @@
rte_align32pow2(BOND_MODE_8023AX_SLAVE_TX_PKTS + 1),
socket_id, 0);
if (port->tx_ring == NULL) {
- rte_panic("Slave %u: Failed to create tx ring '%s': %s\n",
slave_id,
- mem_name, rte_strerror(rte_errno));
+ RTE_LOG(ERR, PMD, "%s() Slave %u: Fail to create tx ring '%s':
%s\n",
+ __func__, slave_id, mem_name, rte_strerror(rte_errno));
+ return -1;
}
+
+ return 0;
}
int
@@ -1143,9 +1148,11 @@
struct bond_dev_private *internals = bond_dev->data->dev_private;
uint8_t i;
- for (i = 0; i < internals->active_slave_count; i++)
- bond_mode_8023ad_activate_slave(bond_dev,
- internals->active_slaves[i]);
+ for (i = 0; i < internals->active_slave_count; i++) {
+ if (!bond_mode_8023ad_activate_slave(bond_dev,
+ internals->active_slaves[i]))
+ return -1;
+ }
return 0;
}
diff --git a/drivers/net/bonding/rte_eth_bond_8023ad_private.h
b/drivers/net/bonding/rte_eth_bond_8023ad_private.h
index 0f490a5..96a42f2 100644
--- a/drivers/net/bonding/rte_eth_bond_8023ad_private.h
+++ b/drivers/net/bonding/rte_eth_bond_8023ad_private.h
@@ -263,7 +263,7 @@ struct mode8023ad_private {
* @return
* 0 on success, negative value otherwise.
*/
-void
+int
bond_mode_8023ad_activate_slave(struct rte_eth_dev *dev, uint16_t port_id);
/**
diff --git a/drivers/net/bonding/rte_eth_bond_api.c
b/drivers/net/bonding/rte_eth_bond_api.c
index aa89425..657fd74 100644
--- a/drivers/net/bonding/rte_eth_bond_api.c
+++ b/drivers/net/bonding/rte_eth_bond_api.c
@@ -69,14 +69,15 @@
return 0;
}
-void
+int
activate_slave(struct rte_eth_dev *eth_dev, uint16_t port_id)
{
struct bond_dev_private *internals = eth_dev->data->dev_private;
uint8_t active_count = internals->active_slave_count;
if (internals->mode == BONDING_MODE_8023AD)
- bond_mode_8023ad_activate_slave(eth_dev, port_id);
+ if (bond_mode_8023ad_activate_slave(eth_dev, port_id) != 0)
+ return -1;
if (internals->mode == B
[dpdk-dev] [PATCH v5 03/11] e1000: replace rte_panic instances in e1000 driver
replace panic calls with log and return value.
Local function to this file,
changing from void to int is non-abi-breaking
Signed-off-by: Arnon Warshavsky
---
drivers/net/e1000/e1000_ethdev.h | 2 +-
drivers/net/e1000/igb_ethdev.c | 4 +++-
drivers/net/e1000/igb_pf.c | 15 +--
3 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/drivers/net/e1000/e1000_ethdev.h b/drivers/net/e1000/e1000_ethdev.h
index 6354b89..2e527de 100644
--- a/drivers/net/e1000/e1000_ethdev.h
+++ b/drivers/net/e1000/e1000_ethdev.h
@@ -411,7 +411,7 @@ int eth_igb_rss_hash_conf_get(struct rte_eth_dev *dev,
/*
* misc function prototypes
*/
-void igb_pf_host_init(struct rte_eth_dev *eth_dev);
+int igb_pf_host_init(struct rte_eth_dev *eth_dev);
void igb_pf_mbx_process(struct rte_eth_dev *eth_dev);
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index 9b808a9..67a32a2 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -833,7 +833,9 @@ static int igb_flex_filter_uninit(struct rte_eth_dev
*eth_dev)
}
/* initialize PF if max_vfs not zero */
- igb_pf_host_init(eth_dev);
+ error = igb_pf_host_init(eth_dev);
+ if (error != 0)
+ goto err_late;
ctrl_ext = E1000_READ_REG(hw, E1000_CTRL_EXT);
/* Set PF Reset Done bit so PF/VF Mail Ops can work */
diff --git a/drivers/net/e1000/igb_pf.c b/drivers/net/e1000/igb_pf.c
index b9f2e53..1a53531 100644
--- a/drivers/net/e1000/igb_pf.c
+++ b/drivers/net/e1000/igb_pf.c
@@ -63,7 +63,7 @@ int igb_vf_perm_addr_gen(struct rte_eth_dev *dev, uint16_t
vf_num)
return 0;
}
-void igb_pf_host_init(struct rte_eth_dev *eth_dev)
+int igb_pf_host_init(struct rte_eth_dev *eth_dev)
{
struct e1000_vf_info **vfinfo =
E1000_DEV_PRIVATE_TO_P_VFDATA(eth_dev->data->dev_private);
@@ -74,7 +74,7 @@ void igb_pf_host_init(struct rte_eth_dev *eth_dev)
RTE_ETH_DEV_SRIOV(eth_dev).active = 0;
if (0 == (vf_num = dev_num_vf(eth_dev)))
- return;
+ return 0;
if (hw->mac.type == e1000_i350)
nb_queue = 1;
@@ -82,11 +82,14 @@ void igb_pf_host_init(struct rte_eth_dev *eth_dev)
/* per datasheet, it should be 2, but 1 seems correct */
nb_queue = 1;
else
- return;
+ return 0;
*vfinfo = rte_zmalloc("vf_info", sizeof(struct e1000_vf_info) * vf_num,
0);
- if (*vfinfo == NULL)
- rte_panic("Cannot allocate memory for private VF data\n");
+ if (*vfinfo == NULL) {
+ RTE_LOG(CRIT, PMD, "%s(): Cannot allocate memory for private VF
data\n",
+ __func__);
+ return -ENOMEM;
+ }
RTE_ETH_DEV_SRIOV(eth_dev).active = ETH_8_POOLS;
RTE_ETH_DEV_SRIOV(eth_dev).nb_q_per_pool = nb_queue;
@@ -98,7 +101,7 @@ void igb_pf_host_init(struct rte_eth_dev *eth_dev)
/* set mb interrupt mask */
igb_mb_intr_setup(eth_dev);
- return;
+ return 0;
}
void igb_pf_host_uninit(struct rte_eth_dev *dev)
--
1.8.3.1
[dpdk-dev] [PATCH v5 04/11] ixgbe: replace rte_panic instances in ixgbe driver
replace panic calls with log and return value.
Local function to this file,
changing from void to int is non-abi-breaking
Signed-off-by: Arnon Warshavsky
---
drivers/net/ixgbe/ixgbe_ethdev.c | 6 --
drivers/net/ixgbe/ixgbe_ethdev.h | 2 +-
drivers/net/ixgbe/ixgbe_pf.c | 15 ++-
3 files changed, 15 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index a5e2fc0..fb95cc7 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -1061,7 +1061,7 @@ struct rte_ixgbe_xstats_name_off {
IXGBE_DEV_PRIVATE_TO_BW_CONF(eth_dev->data->dev_private);
uint32_t ctrl_ext;
uint16_t csum;
- int diag, i;
+ int diag, i, error;
PMD_INIT_FUNC_TRACE();
@@ -1224,7 +1224,9 @@ struct rte_ixgbe_xstats_name_off {
memset(hwstrip, 0, sizeof(*hwstrip));
/* initialize PF if max_vfs not zero */
- ixgbe_pf_host_init(eth_dev);
+ error = ixgbe_pf_host_init(eth_dev);
+ if (error != 0)
+ return error;
ctrl_ext = IXGBE_READ_REG(hw, IXGBE_CTRL_EXT);
/* let hardware know driver is loaded */
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
index 6550777..8bb41ec 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.h
+++ b/drivers/net/ixgbe/ixgbe_ethdev.h
@@ -661,7 +661,7 @@ int ixgbe_fdir_filter_program(struct rte_eth_dev *dev,
void ixgbe_vlan_hw_strip_config(struct rte_eth_dev *dev);
-void ixgbe_pf_host_init(struct rte_eth_dev *eth_dev);
+int ixgbe_pf_host_init(struct rte_eth_dev *eth_dev);
void ixgbe_pf_host_uninit(struct rte_eth_dev *eth_dev);
diff --git a/drivers/net/ixgbe/ixgbe_pf.c b/drivers/net/ixgbe/ixgbe_pf.c
index 4e61310..1e58750 100644
--- a/drivers/net/ixgbe/ixgbe_pf.c
+++ b/drivers/net/ixgbe/ixgbe_pf.c
@@ -66,7 +66,7 @@ int ixgbe_vf_perm_addr_gen(struct rte_eth_dev *dev, uint16_t
vf_num)
return 0;
}
-void ixgbe_pf_host_init(struct rte_eth_dev *eth_dev)
+int ixgbe_pf_host_init(struct rte_eth_dev *eth_dev)
{
struct ixgbe_vf_info **vfinfo =
IXGBE_DEV_PRIVATE_TO_P_VFDATA(eth_dev->data->dev_private);
@@ -84,11 +84,14 @@ void ixgbe_pf_host_init(struct rte_eth_dev *eth_dev)
RTE_ETH_DEV_SRIOV(eth_dev).active = 0;
vf_num = dev_num_vf(eth_dev);
if (vf_num == 0)
- return;
+ return 0;
*vfinfo = rte_zmalloc("vf_info", sizeof(struct ixgbe_vf_info) * vf_num,
0);
- if (*vfinfo == NULL)
- rte_panic("Cannot allocate memory for private VF data\n");
+ if (*vfinfo == NULL) {
+ RTE_LOG(ERR, PMD, "%s() Cannot allocate memory for private VF
data\n",
+ __func__);
+ return -ENOMEM;
+ }
memset(mirror_info, 0, sizeof(struct ixgbe_mirror_info));
memset(uta_info, 0, sizeof(struct ixgbe_uta_info));
@@ -116,6 +119,8 @@ void ixgbe_pf_host_init(struct rte_eth_dev *eth_dev)
/* set mb interrupt mask */
ixgbe_mb_intr_setup(eth_dev);
+
+ return 0;
}
void ixgbe_pf_host_uninit(struct rte_eth_dev *eth_dev)
@@ -203,7 +208,7 @@ int ixgbe_pf_host_configure(struct rte_eth_dev *eth_dev)
vf_num = dev_num_vf(eth_dev);
if (vf_num == 0)
- return -1;
+ return -ENOMEM;
/* enable VMDq and set the default pool for PF */
vtctl = IXGBE_READ_REG(hw, IXGBE_VT_CTL);
--
1.8.3.1
[dpdk-dev] [PATCH v5 05/11] eal: replace rte_panic instances in eventdev
replace panic calls with log and return value.
Signed-off-by: Arnon Warshavsky
---
lib/librte_eventdev/rte_eventdev_pmd_pci.h | 8 +---
lib/librte_eventdev/rte_eventdev_pmd_vdev.h | 8 +---
2 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/lib/librte_eventdev/rte_eventdev_pmd_pci.h
b/lib/librte_eventdev/rte_eventdev_pmd_pci.h
index 8fb6138..6e08705 100644
--- a/lib/librte_eventdev/rte_eventdev_pmd_pci.h
+++ b/lib/librte_eventdev/rte_eventdev_pmd_pci.h
@@ -66,9 +66,11 @@
RTE_CACHE_LINE_SIZE,
rte_socket_id());
- if (eventdev->data->dev_private == NULL)
- rte_panic("Cannot allocate memzone for private "
- "device data");
+ if (eventdev->data->dev_private == NULL) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot allocate memzone for
private device data",
+ __func__);
+ return -ENOMEM;
+ }
}
eventdev->dev = &pci_dev->device;
diff --git a/lib/librte_eventdev/rte_eventdev_pmd_vdev.h
b/lib/librte_eventdev/rte_eventdev_pmd_vdev.h
index 8c64a06..b7c08fa 100644
--- a/lib/librte_eventdev/rte_eventdev_pmd_vdev.h
+++ b/lib/librte_eventdev/rte_eventdev_pmd_vdev.h
@@ -61,9 +61,11 @@
RTE_CACHE_LINE_SIZE,
socket_id);
- if (eventdev->data->dev_private == NULL)
- rte_panic("Cannot allocate memzone for private device"
- " data");
+ if (eventdev->data->dev_private == NULL) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot allocate memzone for
private device data",
+ __func__);
+ return NULL;
+ }
}
return eventdev;
--
1.8.3.1
[dpdk-dev] [PATCH v5 06/11] kni: replace rte_panic instances in kni
replace panic calls with log and return value.
Local function to this file,
changing from void to int is non-abi-breaking
Signed-off-by: Arnon Warshavsky
---
lib/librte_kni/rte_kni.c | 18 --
lib/librte_kni/rte_kni_fifo.h | 11 ---
2 files changed, 20 insertions(+), 9 deletions(-)
diff --git a/lib/librte_kni/rte_kni.c b/lib/librte_kni/rte_kni.c
index 8a8f6c1..4dac407 100644
--- a/lib/librte_kni/rte_kni.c
+++ b/lib/librte_kni/rte_kni.c
@@ -353,37 +353,43 @@ struct rte_kni *
/* TX RING */
mz = slot->m_tx_q;
ctx->tx_q = mz->addr;
- kni_fifo_init(ctx->tx_q, KNI_FIFO_COUNT_MAX);
+ if (kni_fifo_init(ctx->tx_q, KNI_FIFO_COUNT_MAX))
+ return NULL;
dev_info.tx_phys = mz->phys_addr;
/* RX RING */
mz = slot->m_rx_q;
ctx->rx_q = mz->addr;
- kni_fifo_init(ctx->rx_q, KNI_FIFO_COUNT_MAX);
+ if (kni_fifo_init(ctx->rx_q, KNI_FIFO_COUNT_MAX))
+ return NULL;
dev_info.rx_phys = mz->phys_addr;
/* ALLOC RING */
mz = slot->m_alloc_q;
ctx->alloc_q = mz->addr;
- kni_fifo_init(ctx->alloc_q, KNI_FIFO_COUNT_MAX);
+ if (kni_fifo_init(ctx->alloc_q, KNI_FIFO_COUNT_MAX))
+ return NULL;
dev_info.alloc_phys = mz->phys_addr;
/* FREE RING */
mz = slot->m_free_q;
ctx->free_q = mz->addr;
- kni_fifo_init(ctx->free_q, KNI_FIFO_COUNT_MAX);
+ if (kni_fifo_init(ctx->free_q, KNI_FIFO_COUNT_MAX))
+ return NULL;
dev_info.free_phys = mz->phys_addr;
/* Request RING */
mz = slot->m_req_q;
ctx->req_q = mz->addr;
- kni_fifo_init(ctx->req_q, KNI_FIFO_COUNT_MAX);
+ if (kni_fifo_init(ctx->req_q, KNI_FIFO_COUNT_MAX))
+ return NULL;
dev_info.req_phys = mz->phys_addr;
/* Response RING */
mz = slot->m_resp_q;
ctx->resp_q = mz->addr;
- kni_fifo_init(ctx->resp_q, KNI_FIFO_COUNT_MAX);
+ if (kni_fifo_init(ctx->resp_q, KNI_FIFO_COUNT_MAX))
+ return NULL;
dev_info.resp_phys = mz->phys_addr;
/* Req/Resp sync mem area */
diff --git a/lib/librte_kni/rte_kni_fifo.h b/lib/librte_kni/rte_kni_fifo.h
index ac26a8c..5052015 100644
--- a/lib/librte_kni/rte_kni_fifo.h
+++ b/lib/librte_kni/rte_kni_fifo.h
@@ -7,17 +7,22 @@
/**
* Initializes the kni fifo structure
*/
-static void
+static int
kni_fifo_init(struct rte_kni_fifo *fifo, unsigned size)
{
/* Ensure size is power of 2 */
- if (size & (size - 1))
- rte_panic("KNI fifo size must be power of 2\n");
+ if (size & (size - 1)) {
+ RTE_LOG(CRIT, EAL, "%s(): KNI fifo size must be power of 2\n",
+ __func__);
+ return -1;
+ }
fifo->write = 0;
fifo->read = 0;
fifo->len = size;
fifo->elem_size = sizeof(void *);
+
+ return 0;
}
/**
--
1.8.3.1
[dpdk-dev] [PATCH v5 07/11] eal: replace rte_panic instances in hugepage_info
replace panic calls with log and return value.
Signed-off-by: Arnon Warshavsky
---
lib/librte_eal/linuxapp/eal/eal_hugepage_info.c | 37 +
1 file changed, 26 insertions(+), 11 deletions(-)
diff --git a/lib/librte_eal/linuxapp/eal/eal_hugepage_info.c
b/lib/librte_eal/linuxapp/eal/eal_hugepage_info.c
index db5aabd..797b8fa 100644
--- a/lib/librte_eal/linuxapp/eal/eal_hugepage_info.c
+++ b/lib/librte_eal/linuxapp/eal/eal_hugepage_info.c
@@ -145,8 +145,8 @@
return num_pages;
}
-static uint64_t
-get_default_hp_size(void)
+static int
+get_default_hp_size(uint64_t *result)
{
const char proc_meminfo[] = "/proc/meminfo";
const char str_hugepagesz[] = "Hugepagesize:";
@@ -155,8 +155,11 @@
unsigned long long size = 0;
FILE *fd = fopen(proc_meminfo, "r");
- if (fd == NULL)
- rte_panic("Cannot open %s\n", proc_meminfo);
+ if (fd == NULL) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot open %s\n",
+ __func__, proc_meminfo);
+ return -1;
+ }
while(fgets(buffer, sizeof(buffer), fd)){
if (strncmp(buffer, str_hugepagesz, hugepagesz_len) == 0){
size = rte_str_to_size(&buffer[hugepagesz_len]);
@@ -164,9 +167,13 @@
}
}
fclose(fd);
- if (size == 0)
- rte_panic("Cannot get default hugepage size from %s\n",
proc_meminfo);
- return size;
+ if (size == 0) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot get default hugepage size from
%s\n",
+__func__, proc_meminfo);
+ return -1;
+ }
+ *result = size;
+ return 0;
}
static int
@@ -191,11 +198,19 @@
int retval = -1;
FILE *fd = fopen(proc_mounts, "r");
- if (fd == NULL)
- rte_panic("Cannot open %s\n", proc_mounts);
+ if (fd == NULL) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot open %s\n",
+ __func__, proc_mounts);
+ return -ENOENT;
+ }
- if (default_size == 0)
- default_size = get_default_hp_size();
+ if (default_size == 0) {
+ retval = get_default_hp_size(&default_size);
+ if (retval) {
+ fclose(fd);
+ return retval;
+ }
+ }
while (fgets(buf, sizeof(buf), fd)){
if (rte_strsplit(buf, sizeof(buf), splitstr, _FIELDNAME_MAX,
--
1.8.3.1
[dpdk-dev] [PATCH v5 08/11] eal: replace rte_panic instances in interrupts thread
replace panic calls with log and return value.
Thread function removes the noreturn attribute.
Signed-off-by: Arnon Warshavsky
---
lib/librte_eal/linuxapp/eal/eal_interrupts.c | 25 ++---
1 file changed, 18 insertions(+), 7 deletions(-)
diff --git a/lib/librte_eal/linuxapp/eal/eal_interrupts.c
b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
index 58e9328..77e6f2a 100644
--- a/lib/librte_eal/linuxapp/eal/eal_interrupts.c
+++ b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
@@ -785,7 +785,7 @@ struct rte_intr_source {
* @return
* never return;
*/
-static __attribute__((noreturn)) void *
+static void *
eal_intr_thread_main(__rte_unused void *arg)
{
struct epoll_event ev;
@@ -803,8 +803,11 @@ static __attribute__((noreturn)) void *
/* create epoll fd */
int pfd = epoll_create(1);
- if (pfd < 0)
- rte_panic("Cannot create epoll instance\n");
+ if (pfd < 0) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot create epoll
instance\n",
+ __func__);
+ return NULL;
+ }
pipe_event.data.fd = intr_pipe.readfd;
/**
@@ -813,8 +816,11 @@ static __attribute__((noreturn)) void *
*/
if (epoll_ctl(pfd, EPOLL_CTL_ADD, intr_pipe.readfd,
&pipe_event) < 0) {
- rte_panic("Error adding fd to %d epoll_ctl, %s\n",
+ RTE_LOG(CRIT, EAL, "%s(): Error adding fd to %d "
+ "epoll_ctl, %s\n",
+ __func__,
intr_pipe.readfd, strerror(errno));
+ return NULL;
}
numfds++;
@@ -831,9 +837,12 @@ static __attribute__((noreturn)) void *
* into wait list.
*/
if (epoll_ctl(pfd, EPOLL_CTL_ADD,
- src->intr_handle.fd, &ev) < 0){
- rte_panic("Error adding fd %d epoll_ctl, %s\n",
- src->intr_handle.fd, strerror(errno));
+ src->intr_handle.fd, &ev) < 0) {
+ RTE_LOG(CRIT, EAL, "%s(): Error adding fd %d
epoll_ctl, %s\n",
+ __func__,
+ src->intr_handle.fd,
+ strerror(errno));
+ return NULL;
}
else
numfds++;
@@ -848,6 +857,8 @@ static __attribute__((noreturn)) void *
*/
close(pfd);
}
+
+ return NULL;
}
int
--
1.8.3.1
[dpdk-dev] [PATCH v5 10/11] eal: replace rte_panic instances in init sequence
Change some local functions return type from void to int.
This change does not break ABI as the functions are internal.
Panic thrown from threads was not handled in this patch
Signed-off-by: Arnon Warshavsky
---
lib/librte_eal/bsdapp/eal/eal.c | 70
lib/librte_eal/linuxapp/eal/eal.c | 97 ++-
2 files changed, 115 insertions(+), 52 deletions(-)
diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
index d996190..a3c3b37 100644
--- a/lib/librte_eal/bsdapp/eal/eal.c
+++ b/lib/librte_eal/bsdapp/eal/eal.c
@@ -151,7 +151,7 @@ enum rte_iova_mode
* We also don't lock the whole file, so that in future we can use read-locks
* on other parts, e.g. memzones, to detect if there are running secondary
* processes. */
-static void
+static int
rte_eal_config_create(void)
{
void *rte_mem_cfg_addr;
@@ -160,60 +160,81 @@ enum rte_iova_mode
const char *pathname = eal_runtime_config_path();
if (internal_config.no_shconf)
- return;
+ return 0;
if (mem_cfg_fd < 0){
mem_cfg_fd = open(pathname, O_RDWR | O_CREAT, 0660);
- if (mem_cfg_fd < 0)
- rte_panic("Cannot open '%s' for rte_mem_config\n",
pathname);
+ if (mem_cfg_fd < 0) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot open '%s' for
rte_mem_config\n",
+ __func__, pathname);
+ return -1;
+ }
}
retval = ftruncate(mem_cfg_fd, sizeof(*rte_config.mem_config));
if (retval < 0){
close(mem_cfg_fd);
- rte_panic("Cannot resize '%s' for rte_mem_config\n", pathname);
+ mem_cfg_fd = -1;
+ RTE_LOG(CRIT, EAL, "%s(): Cannot resize '%s' for
rte_mem_config\n",
+ __func__, pathname);
+ return -1;
}
retval = fcntl(mem_cfg_fd, F_SETLK, &wr_lock);
if (retval < 0){
close(mem_cfg_fd);
- rte_exit(EXIT_FAILURE, "Cannot create lock on '%s'. Is another
primary "
- "process running?\n", pathname);
+ mem_cfg_fd = -1;
+ RTE_LOG(CRIT, EAL, "%s(): Cannot create lock on '%s'. Is
another primary process running?\n",
+ __func__, pathname);
+ return -1;
}
rte_mem_cfg_addr = mmap(NULL, sizeof(*rte_config.mem_config),
PROT_READ | PROT_WRITE, MAP_SHARED, mem_cfg_fd,
0);
if (rte_mem_cfg_addr == MAP_FAILED){
- rte_panic("Cannot mmap memory for rte_config\n");
+ RTE_LOG(CRIT, EAL, "%s(): Cannot mmap memory for rte_config\n",
+ __func__);
+ return -1;
}
memcpy(rte_mem_cfg_addr, &early_mem_config, sizeof(early_mem_config));
rte_config.mem_config = rte_mem_cfg_addr;
+
+ return 0;
}
/* attach to an existing shared memory config */
-static void
+static int
rte_eal_config_attach(void)
{
void *rte_mem_cfg_addr;
const char *pathname = eal_runtime_config_path();
if (internal_config.no_shconf)
- return;
+ return 0;
if (mem_cfg_fd < 0){
mem_cfg_fd = open(pathname, O_RDWR);
- if (mem_cfg_fd < 0)
- rte_panic("Cannot open '%s' for rte_mem_config\n",
pathname);
+ if (mem_cfg_fd < 0) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot open '%s' for
rte_mem_config\n",
+ __func__, pathname);
+ return -1;
+ }
}
rte_mem_cfg_addr = mmap(NULL, sizeof(*rte_config.mem_config),
PROT_READ | PROT_WRITE, MAP_SHARED, mem_cfg_fd,
0);
close(mem_cfg_fd);
- if (rte_mem_cfg_addr == MAP_FAILED)
- rte_panic("Cannot mmap memory for rte_config\n");
+ if (rte_mem_cfg_addr == MAP_FAILED) {
+ mem_cfg_fd = -1;
+ RTE_LOG(CRIT, EAL, "%s(): Cannot mmap memory for rte_config\n",
+ __func__);
+ return -1;
+ }
rte_config.mem_config = rte_mem_cfg_addr;
+
+ return 0;
}
/* Detect if we are a primary or a secondary process */
@@ -237,23 +258,29 @@ enum rte_proc_type_t
}
/* Sets up rte_config structure with the pointer to shared memory config.*/
-static void
+static int
rte_config_init(void)
{
rte_config.process_type = internal_config.process_type;
[dpdk-dev] [PATCH v5 09/11] eal: replace rte_panic instances in ethdev
Local function to this file,
changing from void to int is non-abi-breaking
Signed-off-by: Arnon Warshavsky
---
lib/librte_ether/rte_ethdev.c | 42 ++
lib/librte_ether/rte_ethdev.h | 4 +++-
2 files changed, 33 insertions(+), 13 deletions(-)
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 7821a88..4ffdc54 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -194,7 +194,7 @@ enum {
return port_id;
}
-static void
+static int
rte_eth_dev_shared_data_prepare(void)
{
const unsigned flags = 0;
@@ -210,8 +210,12 @@ enum {
rte_socket_id(), flags);
} else
mz = rte_memzone_lookup(MZ_RTE_ETH_DEV_DATA);
- if (mz == NULL)
- rte_panic("Cannot allocate ethdev shared data\n");
+ if (mz == NULL) {
+ rte_spinlock_unlock(&rte_eth_shared_data_lock);
+ RTE_LOG(CRIT, EAL, "%s(): Cannot allocate ethdev shared
data\n",
+ __func__);
+ return -1;
+ }
rte_eth_dev_shared_data = mz->addr;
if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
@@ -224,6 +228,8 @@ enum {
}
rte_spinlock_unlock(&rte_eth_shared_data_lock);
+
+ return 0;
}
struct rte_eth_dev *
@@ -274,7 +280,8 @@ struct rte_eth_dev *
uint16_t port_id;
struct rte_eth_dev *eth_dev = NULL;
- rte_eth_dev_shared_data_prepare();
+ if (rte_eth_dev_shared_data_prepare() != 0)
+ return NULL;
/* Synchronize port creation between primary and secondary threads. */
rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
@@ -317,7 +324,8 @@ struct rte_eth_dev *
uint16_t i;
struct rte_eth_dev *eth_dev = NULL;
- rte_eth_dev_shared_data_prepare();
+ if (rte_eth_dev_shared_data_prepare() != 0)
+ return NULL;
/* Synchronize port attachment to primary port creation and release. */
rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
@@ -345,7 +353,8 @@ struct rte_eth_dev *
if (eth_dev == NULL)
return -EINVAL;
- rte_eth_dev_shared_data_prepare();
+ if (rte_eth_dev_shared_data_prepare() != 0)
+ return -1;
rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
@@ -399,7 +408,8 @@ struct rte_eth_dev *
int __rte_experimental
rte_eth_dev_owner_new(uint64_t *owner_id)
{
- rte_eth_dev_shared_data_prepare();
+ if (rte_eth_dev_shared_data_prepare() != 0)
+ return -1;
rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
@@ -450,7 +460,8 @@ struct rte_eth_dev *
{
int ret;
- rte_eth_dev_shared_data_prepare();
+ if (rte_eth_dev_shared_data_prepare() != 0)
+ return -1;
rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
@@ -467,7 +478,8 @@ struct rte_eth_dev *
{.id = RTE_ETH_DEV_NO_OWNER, .name = ""};
int ret;
- rte_eth_dev_shared_data_prepare();
+ if (rte_eth_dev_shared_data_prepare() != 0)
+ return -1;
rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
@@ -477,12 +489,15 @@ struct rte_eth_dev *
return ret;
}
-void __rte_experimental
+int __rte_experimental
rte_eth_dev_owner_delete(const uint64_t owner_id)
{
uint16_t port_id;
+ int error;
- rte_eth_dev_shared_data_prepare();
+ error = rte_eth_dev_shared_data_prepare();
+ if (error != 0)
+ return error;
rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
@@ -495,6 +510,8 @@ struct rte_eth_dev *
}
rte_spinlock_unlock(&rte_eth_dev_shared_data->ownership_lock);
+
+ return 0;
}
int __rte_experimental
@@ -502,7 +519,8 @@ struct rte_eth_dev *
{
int ret = 0;
- rte_eth_dev_shared_data_prepare();
+ if (rte_eth_dev_shared_data_prepare() != 0)
+ return -1;
rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index eb090df..8303b9f 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -1354,8 +1354,10 @@ int __rte_experimental rte_eth_dev_owner_unset(const
uint16_t port_id,
*
* @param owner_id
* The owner identifier.
+ * @return
+ * 0 on success, negative errno value on error.
*/
-void __rte_experimental rte_eth_dev_owner_delete(const uint64_t owner_id);
+int __rte_experimental rte_eth_dev_owner_delete(const uint64_t owner_id);
/**
* @warning
--
1.8.3.1
[dpdk-dev] [PATCH v5 11/11] devtools: prevent new instances of rte_panic and rte_exit
This patch adds a new function that is called
per every checked patch,
and alerts for new instances of rte_panic/rte_exit.
The check excludes comments, and alerts in the case
of a positive balance between additions and removals.
Signed-off-by: Arnon Warshavsky
---
devtools/checkpatches.sh | 95 +++-
1 file changed, 94 insertions(+), 1 deletion(-)
diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
index 7676a6b..48b2685 100755
--- a/devtools/checkpatches.sh
+++ b/devtools/checkpatches.sh
@@ -61,6 +61,91 @@ print_usage () {
END_OF_HELP
}
+check_forbidden_additions() { #
+# -
+#This awk script receives a list of expressions to monitor
+#and a list of folders to search these expressions in
+# - No search is done inside comments
+# - Both additions and removals of the expressions are checked
+# A positive balance of additions fails the check
+ read -d '' awk_script << 'EOF'
+ BEGIN{
+ split(FOLDERS,deny_folders," ");
+ split(EXPRESSIONS,deny_expr," ");
+ in_file=0;
+ in_comment=0;
+ count=0;
+ comment_start="/*"
+ comment_end="*/"
+}
+# search for add/remove instances in current file
+# state machine assumes the comments structure is enforced by
+# checkpatches.pl
+(in_file) {
+ # comment start
+ if (index($0,comment_start) > 0){
+ in_comment = 1
+ }
+ # non comment code
+ if (in_comment == 0) {
+ for (i in deny_expr) {
+ forbidden_added = "^\+.*" deny_expr[i];
+ forbidden_removed="^-.*" deny_expr[i];
+ current = expressions[deny_expr[i]]
+ if ($0 ~ forbidden_added) {
+ count = count + 1;
+ expressions[deny_expr[i]] = current + 1
+ }
+ if ($0 ~ forbidden_removed) {
+ count = count - 1;
+ expressions[deny_expr[i]] = current - 1
+ }
+ }
+ }
+
+ # comment end
+ if (index($0,comment_end) > 0) {
+ in_comment = 0
+ }
+}
+ # switch to next file , check if the balance of add/remove
+ # of previous filehad new additions
+ ($0 ~ "^\+\+\+ b/") {
+ in_file = 0;
+ if (count > 0){
+ exit;
+ }
+ for (i in deny_folders){
+ re = "^\+\+\+ b/" deny_folders[i];
+ if ($0 ~ deny_folders[i]) {
+ in_file = 1
+ last_file = $0
+ }
+ }
+ }
+ END{
+ if (count > 0){
+ print "Warning: in " substr(last_file,6) ":"
+ print "are you sure you want to add the following:"
+ for (key in expressions) {
+ if (expressions[key] > 0) {
+ print key
+ }
+ }
+ exit 1
+ }
+ }
+EOF
+ # -
+
+ # refrain from new additions of rte_panic() and rte_exit()
+ # under lib and net
+ # multiple folders and expressions are separated by spaces
+ awk -v FOLDERS="lib net" \
+ -v EXPRESSIONS="rte_panic\\\( rte_exit\\\(" \
+ "$awk_script" -
+}
+
number=0
quiet=false
verbose=false
@@ -89,11 +174,19 @@ check () { #
total=$(($total + 1))
! $verbose || printf '\n### %s\n\n' "$3"
if [ -n "$1" ] ; then
+ cat "$1" | check_forbidden_additions
+ [ $? -eq 0 ] || return 0
report=$($DPDK_CHECKPATCH_PATH $options "$1" 2>/dev/null)
elif [ -n "$2" ] ; then
- report=$(git format-patch --find-renames --no-stat --stdout -1
$commit |
+ params=$(echo "--find-renames --no-stat --stdout -1")
+ body=$(git format-patch $params $commit)
+ echo "$body" | check_forbidden_additions
+ [ $? -eq 0 ] || return 0
+ report=$(echo "$body" |
$DPDK_CHECKPATCH_PATH $options - 2>/dev/null)
else
+ check_forbidden_additions -
+ [ $? -eq 0 ] || return 0
report=$($DPDK_CHECKPATCH_PATH $options - 2>/dev/null)
fi
[ $? -ne 0 ] || return 0
--
1.8.3.1
Re: [dpdk-dev] [PATCH v5 04/11] ixgbe: replace rte_panic instances in ixgbe driver
> + if (*vfinfo == NULL) {
> > + RTE_LOG(ERR, PMD, "%s() Cannot allocate memory for private
> VF data\n",
> > + __func__);
>
> Please use PMD_DRV_LOG since that has per driver logging control rather
> than global log.
>
Missed that one..Thanks
[dpdk-dev] [PATCH v6 01/11] crypto/dpaa: replace rte_panic instances in crypto/dpaa driver
replace panic calls with log and return value.
Signed-off-by: Arnon Warshavsky
---
drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c | 8 +---
drivers/crypto/dpaa_sec/dpaa_sec.c | 8 +---
2 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
b/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
index 58cbce8..75c50a5 100644
--- a/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
+++ b/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
@@ -2883,9 +2883,11 @@ struct rte_security_ops dpaa2_sec_security_ops = {
RTE_CACHE_LINE_SIZE,
rte_socket_id());
- if (cryptodev->data->dev_private == NULL)
- rte_panic("Cannot allocate memzone for private "
- "device data");
+ if (cryptodev->data->dev_private == NULL) {
+ RTE_LOG(ERR, PMD, "%s() Cannot allocate memzone for
private device data",
+ __func__);
+ return -ENOMEM;
+ }
}
dpaa2_dev->cryptodev = cryptodev;
diff --git a/drivers/crypto/dpaa_sec/dpaa_sec.c
b/drivers/crypto/dpaa_sec/dpaa_sec.c
index e456fd5..025f844 100644
--- a/drivers/crypto/dpaa_sec/dpaa_sec.c
+++ b/drivers/crypto/dpaa_sec/dpaa_sec.c
@@ -2384,9 +2384,11 @@ struct rte_security_ops dpaa_sec_security_ops = {
RTE_CACHE_LINE_SIZE,
rte_socket_id());
- if (cryptodev->data->dev_private == NULL)
- rte_panic("Cannot allocate memzone for private "
- "device data");
+ if (cryptodev->data->dev_private == NULL) {
+ RTE_LOG(ERR, PMD, "%s() Cannot allocate memzone for
private device data",
+ __func__);
+ return -ENOMEM;
+ }
}
dpaa_dev->crypto_dev = cryptodev;
--
1.8.3.1
[dpdk-dev] [PATCH v6 00/11] al: replace calls to rte_panic and refrain from new instances
The purpose of this patch series is to cleanup the library code from paths that end up aborting the process, and move to checking error values, in order to allow the running process perform an orderly teardown or other mitigation of the event. This patch modifies the majority of rte_panic calls under lib and drivers, and replaces them with a log message and an error return code according to context, that can be propagated up the call stack. - Focus was given to the dpdk initialization path - Some of the panic calls within drivers were left in place where the call is from within an interrupt or calls that are on the data path,where there is no simple applicative route to propagate the error to temination. These should be handled by the driver maintainers.. - local void functions with no api were changed to retrun a value where needed - No change took place in example and test files - No change took place for debug assertions calling panic - A new function was added to devtools/checkpatches.sh in order to prevent new additions of calls to rte_panic under lib and drivers. Keep calm and don't panic --- v2: - reformat error messages so that literal string are in the same line - fix typo in commit message - add new return code to doxigen of rte_memzone_free() v3: - submit all 13 patches changed and unchanged in the same patchset v4: - remove 2 patches that are no more relevant - fix split literal string in error message - change return value -1 to enum - split value and success code in a static function v5: - reword commit messages - revert thread related instances back to panicing - handle file descriptors with state to reset after eal init failure in case re initialization takes place v6: - Use pmd log macro rather than rte_log Arnon Warshavsky (11): crypto/dpaa: replace rte_panic instances in crypto/dpaa driver bond: replace rte_panic instances in bonding driver e1000: replace rte_panic instances in e1000 driver ixgbe: replace rte_panic instances in ixgbe driver eal: replace rte_panic instances in eventdev kni: replace rte_panic instances in kni eal: replace rte_panic instances in hugepage_info eal: replace rte_panic instances in interrupts thread eal: replace rte_panic instances in ethdev eal: replace rte_panic instances in init sequence devtools: prevent new instances of rte_panic and rte_exit devtools/checkpatches.sh | 95 +- drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c | 8 +- drivers/crypto/dpaa_sec/dpaa_sec.c| 8 +- drivers/net/bonding/rte_eth_bond_8023ad.c | 29 --- drivers/net/bonding/rte_eth_bond_8023ad_private.h | 2 +- drivers/net/bonding/rte_eth_bond_api.c| 22 +++-- drivers/net/bonding/rte_eth_bond_pmd.c| 9 ++- drivers/net/bonding/rte_eth_bond_private.h| 2 +- drivers/net/e1000/e1000_ethdev.h | 2 +- drivers/net/e1000/igb_ethdev.c| 4 +- drivers/net/e1000/igb_pf.c| 15 ++-- drivers/net/ixgbe/ixgbe_ethdev.c | 6 +- drivers/net/ixgbe/ixgbe_ethdev.h | 2 +- drivers/net/ixgbe/ixgbe_pf.c | 15 ++-- lib/librte_eal/bsdapp/eal/eal.c | 70 +++- lib/librte_eal/linuxapp/eal/eal.c | 97 +++ lib/librte_eal/linuxapp/eal/eal_hugepage_info.c | 37 ++--- lib/librte_eal/linuxapp/eal/eal_interrupts.c | 25 -- lib/librte_ether/rte_ethdev.c | 42 +++--- lib/librte_ether/rte_ethdev.h | 4 +- lib/librte_eventdev/rte_eventdev_pmd_pci.h| 8 +- lib/librte_eventdev/rte_eventdev_pmd_vdev.h | 8 +- lib/librte_kni/rte_kni.c | 18 +++-- lib/librte_kni/rte_kni_fifo.h | 11 ++- 24 files changed, 396 insertions(+), 143 deletions(-) -- 1.8.3.1
[dpdk-dev] [PATCH v6 03/11] e1000: replace rte_panic instances in e1000 driver
replace panic calls with log and return value.
Local function to this file,
changing from void to int is non-abi-breaking
Signed-off-by: Arnon Warshavsky
---
v6:
- replaced rte_log with pmd log macro
drivers/net/e1000/e1000_ethdev.h | 2 +-
drivers/net/e1000/igb_ethdev.c | 4 +++-
drivers/net/e1000/igb_pf.c | 15 +--
3 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/drivers/net/e1000/e1000_ethdev.h b/drivers/net/e1000/e1000_ethdev.h
index 6354b89..2e527de 100644
--- a/drivers/net/e1000/e1000_ethdev.h
+++ b/drivers/net/e1000/e1000_ethdev.h
@@ -411,7 +411,7 @@ int eth_igb_rss_hash_conf_get(struct rte_eth_dev *dev,
/*
* misc function prototypes
*/
-void igb_pf_host_init(struct rte_eth_dev *eth_dev);
+int igb_pf_host_init(struct rte_eth_dev *eth_dev);
void igb_pf_mbx_process(struct rte_eth_dev *eth_dev);
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index 9b808a9..67a32a2 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -833,7 +833,9 @@ static int igb_flex_filter_uninit(struct rte_eth_dev
*eth_dev)
}
/* initialize PF if max_vfs not zero */
- igb_pf_host_init(eth_dev);
+ error = igb_pf_host_init(eth_dev);
+ if (error != 0)
+ goto err_late;
ctrl_ext = E1000_READ_REG(hw, E1000_CTRL_EXT);
/* Set PF Reset Done bit so PF/VF Mail Ops can work */
diff --git a/drivers/net/e1000/igb_pf.c b/drivers/net/e1000/igb_pf.c
index b9f2e53..6e511a9 100644
--- a/drivers/net/e1000/igb_pf.c
+++ b/drivers/net/e1000/igb_pf.c
@@ -63,7 +63,7 @@ int igb_vf_perm_addr_gen(struct rte_eth_dev *dev, uint16_t
vf_num)
return 0;
}
-void igb_pf_host_init(struct rte_eth_dev *eth_dev)
+int igb_pf_host_init(struct rte_eth_dev *eth_dev)
{
struct e1000_vf_info **vfinfo =
E1000_DEV_PRIVATE_TO_P_VFDATA(eth_dev->data->dev_private);
@@ -74,7 +74,7 @@ void igb_pf_host_init(struct rte_eth_dev *eth_dev)
RTE_ETH_DEV_SRIOV(eth_dev).active = 0;
if (0 == (vf_num = dev_num_vf(eth_dev)))
- return;
+ return 0;
if (hw->mac.type == e1000_i350)
nb_queue = 1;
@@ -82,11 +82,14 @@ void igb_pf_host_init(struct rte_eth_dev *eth_dev)
/* per datasheet, it should be 2, but 1 seems correct */
nb_queue = 1;
else
- return;
+ return 0;
*vfinfo = rte_zmalloc("vf_info", sizeof(struct e1000_vf_info) * vf_num,
0);
- if (*vfinfo == NULL)
- rte_panic("Cannot allocate memory for private VF data\n");
+ if (*vfinfo == NULL) {
+ PMD_DRV_LOG(CRIT, "%s(): Cannot allocate memory for private VF
data\n",
+ __func__);
+ return -ENOMEM;
+ }
RTE_ETH_DEV_SRIOV(eth_dev).active = ETH_8_POOLS;
RTE_ETH_DEV_SRIOV(eth_dev).nb_q_per_pool = nb_queue;
@@ -98,7 +101,7 @@ void igb_pf_host_init(struct rte_eth_dev *eth_dev)
/* set mb interrupt mask */
igb_mb_intr_setup(eth_dev);
- return;
+ return 0;
}
void igb_pf_host_uninit(struct rte_eth_dev *dev)
--
1.8.3.1
[dpdk-dev] [PATCH v6 02/11] bond: replace rte_panic instances in bonding driver
replace panic calls with log and return value.
Local functions to this file,
changing from void to int are non-abi-breaking
Signed-off-by: Arnon Warshavsky
---
drivers/net/bonding/rte_eth_bond_8023ad.c | 29 ++-
drivers/net/bonding/rte_eth_bond_8023ad_private.h | 2 +-
drivers/net/bonding/rte_eth_bond_api.c| 22 -
drivers/net/bonding/rte_eth_bond_pmd.c| 9 ---
drivers/net/bonding/rte_eth_bond_private.h| 2 +-
5 files changed, 42 insertions(+), 22 deletions(-)
diff --git a/drivers/net/bonding/rte_eth_bond_8023ad.c
b/drivers/net/bonding/rte_eth_bond_8023ad.c
index c452318..2fb6cad 100644
--- a/drivers/net/bonding/rte_eth_bond_8023ad.c
+++ b/drivers/net/bonding/rte_eth_bond_8023ad.c
@@ -893,7 +893,7 @@
bond_mode_8023ad_periodic_cb, arg);
}
-void
+int
bond_mode_8023ad_activate_slave(struct rte_eth_dev *bond_dev,
uint16_t slave_id)
{
@@ -939,7 +939,7 @@
timer_cancel(&port->warning_timer);
if (port->mbuf_pool != NULL)
- return;
+ return 0;
RTE_ASSERT(port->rx_ring == NULL);
RTE_ASSERT(port->tx_ring == NULL);
@@ -968,8 +968,9 @@
/* Any memory allocation failure in initialization is critical because
* resources can't be free, so reinitialization is impossible. */
if (port->mbuf_pool == NULL) {
- rte_panic("Slave %u: Failed to create memory pool '%s': %s\n",
- slave_id, mem_name, rte_strerror(rte_errno));
+ RTE_LOG(ERR, PMD, "%s() Slave %u: Failed to create memory pool
'%s': %s\n",
+ __func__, slave_id, mem_name, rte_strerror(rte_errno));
+ return -1;
}
snprintf(mem_name, RTE_DIM(mem_name), "slave_%u_rx", slave_id);
@@ -977,8 +978,9 @@
rte_align32pow2(BOND_MODE_8023AX_SLAVE_RX_PKTS),
socket_id, 0);
if (port->rx_ring == NULL) {
- rte_panic("Slave %u: Failed to create rx ring '%s': %s\n",
slave_id,
- mem_name, rte_strerror(rte_errno));
+ RTE_LOG(ERR, PMD, "%s() Slave %u: Failed to create rx ring
'%s': %s\n",
+ __func__, slave_id, mem_name, rte_strerror(rte_errno));
+ return -1;
}
/* TX ring is at least one pkt longer to make room for marker packet. */
@@ -987,9 +989,12 @@
rte_align32pow2(BOND_MODE_8023AX_SLAVE_TX_PKTS + 1),
socket_id, 0);
if (port->tx_ring == NULL) {
- rte_panic("Slave %u: Failed to create tx ring '%s': %s\n",
slave_id,
- mem_name, rte_strerror(rte_errno));
+ RTE_LOG(ERR, PMD, "%s() Slave %u: Fail to create tx ring '%s':
%s\n",
+ __func__, slave_id, mem_name, rte_strerror(rte_errno));
+ return -1;
}
+
+ return 0;
}
int
@@ -1143,9 +1148,11 @@
struct bond_dev_private *internals = bond_dev->data->dev_private;
uint8_t i;
- for (i = 0; i < internals->active_slave_count; i++)
- bond_mode_8023ad_activate_slave(bond_dev,
- internals->active_slaves[i]);
+ for (i = 0; i < internals->active_slave_count; i++) {
+ if (!bond_mode_8023ad_activate_slave(bond_dev,
+ internals->active_slaves[i]))
+ return -1;
+ }
return 0;
}
diff --git a/drivers/net/bonding/rte_eth_bond_8023ad_private.h
b/drivers/net/bonding/rte_eth_bond_8023ad_private.h
index 0f490a5..96a42f2 100644
--- a/drivers/net/bonding/rte_eth_bond_8023ad_private.h
+++ b/drivers/net/bonding/rte_eth_bond_8023ad_private.h
@@ -263,7 +263,7 @@ struct mode8023ad_private {
* @return
* 0 on success, negative value otherwise.
*/
-void
+int
bond_mode_8023ad_activate_slave(struct rte_eth_dev *dev, uint16_t port_id);
/**
diff --git a/drivers/net/bonding/rte_eth_bond_api.c
b/drivers/net/bonding/rte_eth_bond_api.c
index aa89425..657fd74 100644
--- a/drivers/net/bonding/rte_eth_bond_api.c
+++ b/drivers/net/bonding/rte_eth_bond_api.c
@@ -69,14 +69,15 @@
return 0;
}
-void
+int
activate_slave(struct rte_eth_dev *eth_dev, uint16_t port_id)
{
struct bond_dev_private *internals = eth_dev->data->dev_private;
uint8_t active_count = internals->active_slave_count;
if (internals->mode == BONDING_MODE_8023AD)
- bond_mode_8023ad_activate_slave(eth_dev, port_id);
+ if (bond_mode_8023ad_activate_slave(eth_dev, port_id) != 0)
+ return -1;
if (internals->mode == B
[dpdk-dev] [PATCH v6 06/11] kni: replace rte_panic instances in kni
replace panic calls with log and return value.
Local function to this file,
changing from void to int is non-abi-breaking
Signed-off-by: Arnon Warshavsky
---
lib/librte_kni/rte_kni.c | 18 --
lib/librte_kni/rte_kni_fifo.h | 11 ---
2 files changed, 20 insertions(+), 9 deletions(-)
diff --git a/lib/librte_kni/rte_kni.c b/lib/librte_kni/rte_kni.c
index 8a8f6c1..4dac407 100644
--- a/lib/librte_kni/rte_kni.c
+++ b/lib/librte_kni/rte_kni.c
@@ -353,37 +353,43 @@ struct rte_kni *
/* TX RING */
mz = slot->m_tx_q;
ctx->tx_q = mz->addr;
- kni_fifo_init(ctx->tx_q, KNI_FIFO_COUNT_MAX);
+ if (kni_fifo_init(ctx->tx_q, KNI_FIFO_COUNT_MAX))
+ return NULL;
dev_info.tx_phys = mz->phys_addr;
/* RX RING */
mz = slot->m_rx_q;
ctx->rx_q = mz->addr;
- kni_fifo_init(ctx->rx_q, KNI_FIFO_COUNT_MAX);
+ if (kni_fifo_init(ctx->rx_q, KNI_FIFO_COUNT_MAX))
+ return NULL;
dev_info.rx_phys = mz->phys_addr;
/* ALLOC RING */
mz = slot->m_alloc_q;
ctx->alloc_q = mz->addr;
- kni_fifo_init(ctx->alloc_q, KNI_FIFO_COUNT_MAX);
+ if (kni_fifo_init(ctx->alloc_q, KNI_FIFO_COUNT_MAX))
+ return NULL;
dev_info.alloc_phys = mz->phys_addr;
/* FREE RING */
mz = slot->m_free_q;
ctx->free_q = mz->addr;
- kni_fifo_init(ctx->free_q, KNI_FIFO_COUNT_MAX);
+ if (kni_fifo_init(ctx->free_q, KNI_FIFO_COUNT_MAX))
+ return NULL;
dev_info.free_phys = mz->phys_addr;
/* Request RING */
mz = slot->m_req_q;
ctx->req_q = mz->addr;
- kni_fifo_init(ctx->req_q, KNI_FIFO_COUNT_MAX);
+ if (kni_fifo_init(ctx->req_q, KNI_FIFO_COUNT_MAX))
+ return NULL;
dev_info.req_phys = mz->phys_addr;
/* Response RING */
mz = slot->m_resp_q;
ctx->resp_q = mz->addr;
- kni_fifo_init(ctx->resp_q, KNI_FIFO_COUNT_MAX);
+ if (kni_fifo_init(ctx->resp_q, KNI_FIFO_COUNT_MAX))
+ return NULL;
dev_info.resp_phys = mz->phys_addr;
/* Req/Resp sync mem area */
diff --git a/lib/librte_kni/rte_kni_fifo.h b/lib/librte_kni/rte_kni_fifo.h
index ac26a8c..5052015 100644
--- a/lib/librte_kni/rte_kni_fifo.h
+++ b/lib/librte_kni/rte_kni_fifo.h
@@ -7,17 +7,22 @@
/**
* Initializes the kni fifo structure
*/
-static void
+static int
kni_fifo_init(struct rte_kni_fifo *fifo, unsigned size)
{
/* Ensure size is power of 2 */
- if (size & (size - 1))
- rte_panic("KNI fifo size must be power of 2\n");
+ if (size & (size - 1)) {
+ RTE_LOG(CRIT, EAL, "%s(): KNI fifo size must be power of 2\n",
+ __func__);
+ return -1;
+ }
fifo->write = 0;
fifo->read = 0;
fifo->len = size;
fifo->elem_size = sizeof(void *);
+
+ return 0;
}
/**
--
1.8.3.1
[dpdk-dev] [PATCH v6 05/11] eal: replace rte_panic instances in eventdev
replace panic calls with log and return value.
Signed-off-by: Arnon Warshavsky
---
lib/librte_eventdev/rte_eventdev_pmd_pci.h | 8 +---
lib/librte_eventdev/rte_eventdev_pmd_vdev.h | 8 +---
2 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/lib/librte_eventdev/rte_eventdev_pmd_pci.h
b/lib/librte_eventdev/rte_eventdev_pmd_pci.h
index 8fb6138..6e08705 100644
--- a/lib/librte_eventdev/rte_eventdev_pmd_pci.h
+++ b/lib/librte_eventdev/rte_eventdev_pmd_pci.h
@@ -66,9 +66,11 @@
RTE_CACHE_LINE_SIZE,
rte_socket_id());
- if (eventdev->data->dev_private == NULL)
- rte_panic("Cannot allocate memzone for private "
- "device data");
+ if (eventdev->data->dev_private == NULL) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot allocate memzone for
private device data",
+ __func__);
+ return -ENOMEM;
+ }
}
eventdev->dev = &pci_dev->device;
diff --git a/lib/librte_eventdev/rte_eventdev_pmd_vdev.h
b/lib/librte_eventdev/rte_eventdev_pmd_vdev.h
index 8c64a06..b7c08fa 100644
--- a/lib/librte_eventdev/rte_eventdev_pmd_vdev.h
+++ b/lib/librte_eventdev/rte_eventdev_pmd_vdev.h
@@ -61,9 +61,11 @@
RTE_CACHE_LINE_SIZE,
socket_id);
- if (eventdev->data->dev_private == NULL)
- rte_panic("Cannot allocate memzone for private device"
- " data");
+ if (eventdev->data->dev_private == NULL) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot allocate memzone for
private device data",
+ __func__);
+ return NULL;
+ }
}
return eventdev;
--
1.8.3.1
[dpdk-dev] [PATCH v6 04/11] ixgbe: replace rte_panic instances in ixgbe driver
replace panic calls with log and return value.
Local function to this file,
changing from void to int is non-abi-breaking
Signed-off-by: Arnon Warshavsky
---
v6:
- replaced rte_log with pmd log macro
drivers/net/ixgbe/ixgbe_ethdev.c | 6 --
drivers/net/ixgbe/ixgbe_ethdev.h | 2 +-
drivers/net/ixgbe/ixgbe_pf.c | 15 ++-
3 files changed, 15 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index a5e2fc0..fb95cc7 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -1061,7 +1061,7 @@ struct rte_ixgbe_xstats_name_off {
IXGBE_DEV_PRIVATE_TO_BW_CONF(eth_dev->data->dev_private);
uint32_t ctrl_ext;
uint16_t csum;
- int diag, i;
+ int diag, i, error;
PMD_INIT_FUNC_TRACE();
@@ -1224,7 +1224,9 @@ struct rte_ixgbe_xstats_name_off {
memset(hwstrip, 0, sizeof(*hwstrip));
/* initialize PF if max_vfs not zero */
- ixgbe_pf_host_init(eth_dev);
+ error = ixgbe_pf_host_init(eth_dev);
+ if (error != 0)
+ return error;
ctrl_ext = IXGBE_READ_REG(hw, IXGBE_CTRL_EXT);
/* let hardware know driver is loaded */
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
index 6550777..8bb41ec 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.h
+++ b/drivers/net/ixgbe/ixgbe_ethdev.h
@@ -661,7 +661,7 @@ int ixgbe_fdir_filter_program(struct rte_eth_dev *dev,
void ixgbe_vlan_hw_strip_config(struct rte_eth_dev *dev);
-void ixgbe_pf_host_init(struct rte_eth_dev *eth_dev);
+int ixgbe_pf_host_init(struct rte_eth_dev *eth_dev);
void ixgbe_pf_host_uninit(struct rte_eth_dev *eth_dev);
diff --git a/drivers/net/ixgbe/ixgbe_pf.c b/drivers/net/ixgbe/ixgbe_pf.c
index 4e61310..81a9910 100644
--- a/drivers/net/ixgbe/ixgbe_pf.c
+++ b/drivers/net/ixgbe/ixgbe_pf.c
@@ -66,7 +66,7 @@ int ixgbe_vf_perm_addr_gen(struct rte_eth_dev *dev, uint16_t
vf_num)
return 0;
}
-void ixgbe_pf_host_init(struct rte_eth_dev *eth_dev)
+int ixgbe_pf_host_init(struct rte_eth_dev *eth_dev)
{
struct ixgbe_vf_info **vfinfo =
IXGBE_DEV_PRIVATE_TO_P_VFDATA(eth_dev->data->dev_private);
@@ -84,11 +84,14 @@ void ixgbe_pf_host_init(struct rte_eth_dev *eth_dev)
RTE_ETH_DEV_SRIOV(eth_dev).active = 0;
vf_num = dev_num_vf(eth_dev);
if (vf_num == 0)
- return;
+ return 0;
*vfinfo = rte_zmalloc("vf_info", sizeof(struct ixgbe_vf_info) * vf_num,
0);
- if (*vfinfo == NULL)
- rte_panic("Cannot allocate memory for private VF data\n");
+ if (*vfinfo == NULL) {
+ PMD_DRV_LOG(ERR, "%s() Cannot allocate memory for private VF
data\n",
+ __func__);
+ return -ENOMEM;
+ }
memset(mirror_info, 0, sizeof(struct ixgbe_mirror_info));
memset(uta_info, 0, sizeof(struct ixgbe_uta_info));
@@ -116,6 +119,8 @@ void ixgbe_pf_host_init(struct rte_eth_dev *eth_dev)
/* set mb interrupt mask */
ixgbe_mb_intr_setup(eth_dev);
+
+ return 0;
}
void ixgbe_pf_host_uninit(struct rte_eth_dev *eth_dev)
@@ -203,7 +208,7 @@ int ixgbe_pf_host_configure(struct rte_eth_dev *eth_dev)
vf_num = dev_num_vf(eth_dev);
if (vf_num == 0)
- return -1;
+ return -ENOMEM;
/* enable VMDq and set the default pool for PF */
vtctl = IXGBE_READ_REG(hw, IXGBE_VT_CTL);
--
1.8.3.1
[dpdk-dev] [PATCH v6 11/11] devtools: prevent new instances of rte_panic and rte_exit
This patch adds a new function that is called
per every checked patch,
and alerts for new instances of rte_panic/rte_exit.
The check excludes comments, and alerts in the case
of a positive balance between additions and removals.
Signed-off-by: Arnon Warshavsky
---
devtools/checkpatches.sh | 95 +++-
1 file changed, 94 insertions(+), 1 deletion(-)
diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
index 7676a6b..48b2685 100755
--- a/devtools/checkpatches.sh
+++ b/devtools/checkpatches.sh
@@ -61,6 +61,91 @@ print_usage () {
END_OF_HELP
}
+check_forbidden_additions() { #
+# -
+#This awk script receives a list of expressions to monitor
+#and a list of folders to search these expressions in
+# - No search is done inside comments
+# - Both additions and removals of the expressions are checked
+# A positive balance of additions fails the check
+ read -d '' awk_script << 'EOF'
+ BEGIN{
+ split(FOLDERS,deny_folders," ");
+ split(EXPRESSIONS,deny_expr," ");
+ in_file=0;
+ in_comment=0;
+ count=0;
+ comment_start="/*"
+ comment_end="*/"
+}
+# search for add/remove instances in current file
+# state machine assumes the comments structure is enforced by
+# checkpatches.pl
+(in_file) {
+ # comment start
+ if (index($0,comment_start) > 0){
+ in_comment = 1
+ }
+ # non comment code
+ if (in_comment == 0) {
+ for (i in deny_expr) {
+ forbidden_added = "^\+.*" deny_expr[i];
+ forbidden_removed="^-.*" deny_expr[i];
+ current = expressions[deny_expr[i]]
+ if ($0 ~ forbidden_added) {
+ count = count + 1;
+ expressions[deny_expr[i]] = current + 1
+ }
+ if ($0 ~ forbidden_removed) {
+ count = count - 1;
+ expressions[deny_expr[i]] = current - 1
+ }
+ }
+ }
+
+ # comment end
+ if (index($0,comment_end) > 0) {
+ in_comment = 0
+ }
+}
+ # switch to next file , check if the balance of add/remove
+ # of previous filehad new additions
+ ($0 ~ "^\+\+\+ b/") {
+ in_file = 0;
+ if (count > 0){
+ exit;
+ }
+ for (i in deny_folders){
+ re = "^\+\+\+ b/" deny_folders[i];
+ if ($0 ~ deny_folders[i]) {
+ in_file = 1
+ last_file = $0
+ }
+ }
+ }
+ END{
+ if (count > 0){
+ print "Warning: in " substr(last_file,6) ":"
+ print "are you sure you want to add the following:"
+ for (key in expressions) {
+ if (expressions[key] > 0) {
+ print key
+ }
+ }
+ exit 1
+ }
+ }
+EOF
+ # -
+
+ # refrain from new additions of rte_panic() and rte_exit()
+ # under lib and net
+ # multiple folders and expressions are separated by spaces
+ awk -v FOLDERS="lib net" \
+ -v EXPRESSIONS="rte_panic\\\( rte_exit\\\(" \
+ "$awk_script" -
+}
+
number=0
quiet=false
verbose=false
@@ -89,11 +174,19 @@ check () { #
total=$(($total + 1))
! $verbose || printf '\n### %s\n\n' "$3"
if [ -n "$1" ] ; then
+ cat "$1" | check_forbidden_additions
+ [ $? -eq 0 ] || return 0
report=$($DPDK_CHECKPATCH_PATH $options "$1" 2>/dev/null)
elif [ -n "$2" ] ; then
- report=$(git format-patch --find-renames --no-stat --stdout -1
$commit |
+ params=$(echo "--find-renames --no-stat --stdout -1")
+ body=$(git format-patch $params $commit)
+ echo "$body" | check_forbidden_additions
+ [ $? -eq 0 ] || return 0
+ report=$(echo "$body" |
$DPDK_CHECKPATCH_PATH $options - 2>/dev/null)
else
+ check_forbidden_additions -
+ [ $? -eq 0 ] || return 0
report=$($DPDK_CHECKPATCH_PATH $options - 2>/dev/null)
fi
[ $? -ne 0 ] || return 0
--
1.8.3.1
[dpdk-dev] [PATCH v6 09/11] eal: replace rte_panic instances in ethdev
Local function to this file,
changing from void to int is non-abi-breaking
Signed-off-by: Arnon Warshavsky
---
lib/librte_ether/rte_ethdev.c | 42 ++
lib/librte_ether/rte_ethdev.h | 4 +++-
2 files changed, 33 insertions(+), 13 deletions(-)
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 7821a88..4ffdc54 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -194,7 +194,7 @@ enum {
return port_id;
}
-static void
+static int
rte_eth_dev_shared_data_prepare(void)
{
const unsigned flags = 0;
@@ -210,8 +210,12 @@ enum {
rte_socket_id(), flags);
} else
mz = rte_memzone_lookup(MZ_RTE_ETH_DEV_DATA);
- if (mz == NULL)
- rte_panic("Cannot allocate ethdev shared data\n");
+ if (mz == NULL) {
+ rte_spinlock_unlock(&rte_eth_shared_data_lock);
+ RTE_LOG(CRIT, EAL, "%s(): Cannot allocate ethdev shared
data\n",
+ __func__);
+ return -1;
+ }
rte_eth_dev_shared_data = mz->addr;
if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
@@ -224,6 +228,8 @@ enum {
}
rte_spinlock_unlock(&rte_eth_shared_data_lock);
+
+ return 0;
}
struct rte_eth_dev *
@@ -274,7 +280,8 @@ struct rte_eth_dev *
uint16_t port_id;
struct rte_eth_dev *eth_dev = NULL;
- rte_eth_dev_shared_data_prepare();
+ if (rte_eth_dev_shared_data_prepare() != 0)
+ return NULL;
/* Synchronize port creation between primary and secondary threads. */
rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
@@ -317,7 +324,8 @@ struct rte_eth_dev *
uint16_t i;
struct rte_eth_dev *eth_dev = NULL;
- rte_eth_dev_shared_data_prepare();
+ if (rte_eth_dev_shared_data_prepare() != 0)
+ return NULL;
/* Synchronize port attachment to primary port creation and release. */
rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
@@ -345,7 +353,8 @@ struct rte_eth_dev *
if (eth_dev == NULL)
return -EINVAL;
- rte_eth_dev_shared_data_prepare();
+ if (rte_eth_dev_shared_data_prepare() != 0)
+ return -1;
rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
@@ -399,7 +408,8 @@ struct rte_eth_dev *
int __rte_experimental
rte_eth_dev_owner_new(uint64_t *owner_id)
{
- rte_eth_dev_shared_data_prepare();
+ if (rte_eth_dev_shared_data_prepare() != 0)
+ return -1;
rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
@@ -450,7 +460,8 @@ struct rte_eth_dev *
{
int ret;
- rte_eth_dev_shared_data_prepare();
+ if (rte_eth_dev_shared_data_prepare() != 0)
+ return -1;
rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
@@ -467,7 +478,8 @@ struct rte_eth_dev *
{.id = RTE_ETH_DEV_NO_OWNER, .name = ""};
int ret;
- rte_eth_dev_shared_data_prepare();
+ if (rte_eth_dev_shared_data_prepare() != 0)
+ return -1;
rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
@@ -477,12 +489,15 @@ struct rte_eth_dev *
return ret;
}
-void __rte_experimental
+int __rte_experimental
rte_eth_dev_owner_delete(const uint64_t owner_id)
{
uint16_t port_id;
+ int error;
- rte_eth_dev_shared_data_prepare();
+ error = rte_eth_dev_shared_data_prepare();
+ if (error != 0)
+ return error;
rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
@@ -495,6 +510,8 @@ struct rte_eth_dev *
}
rte_spinlock_unlock(&rte_eth_dev_shared_data->ownership_lock);
+
+ return 0;
}
int __rte_experimental
@@ -502,7 +519,8 @@ struct rte_eth_dev *
{
int ret = 0;
- rte_eth_dev_shared_data_prepare();
+ if (rte_eth_dev_shared_data_prepare() != 0)
+ return -1;
rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index b9eb8ae..46e5947 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -1354,8 +1354,10 @@ int __rte_experimental rte_eth_dev_owner_unset(const
uint16_t port_id,
*
* @param owner_id
* The owner identifier.
+ * @return
+ * 0 on success, negative errno value on error.
*/
-void __rte_experimental rte_eth_dev_owner_delete(const uint64_t owner_id);
+int __rte_experimental rte_eth_dev_owner_delete(const uint64_t owner_id);
/**
* @warning
--
1.8.3.1
[dpdk-dev] [PATCH v6 07/11] eal: replace rte_panic instances in hugepage_info
replace panic calls with log and return value.
Signed-off-by: Arnon Warshavsky
---
lib/librte_eal/linuxapp/eal/eal_hugepage_info.c | 37 +
1 file changed, 26 insertions(+), 11 deletions(-)
diff --git a/lib/librte_eal/linuxapp/eal/eal_hugepage_info.c
b/lib/librte_eal/linuxapp/eal/eal_hugepage_info.c
index db5aabd..797b8fa 100644
--- a/lib/librte_eal/linuxapp/eal/eal_hugepage_info.c
+++ b/lib/librte_eal/linuxapp/eal/eal_hugepage_info.c
@@ -145,8 +145,8 @@
return num_pages;
}
-static uint64_t
-get_default_hp_size(void)
+static int
+get_default_hp_size(uint64_t *result)
{
const char proc_meminfo[] = "/proc/meminfo";
const char str_hugepagesz[] = "Hugepagesize:";
@@ -155,8 +155,11 @@
unsigned long long size = 0;
FILE *fd = fopen(proc_meminfo, "r");
- if (fd == NULL)
- rte_panic("Cannot open %s\n", proc_meminfo);
+ if (fd == NULL) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot open %s\n",
+ __func__, proc_meminfo);
+ return -1;
+ }
while(fgets(buffer, sizeof(buffer), fd)){
if (strncmp(buffer, str_hugepagesz, hugepagesz_len) == 0){
size = rte_str_to_size(&buffer[hugepagesz_len]);
@@ -164,9 +167,13 @@
}
}
fclose(fd);
- if (size == 0)
- rte_panic("Cannot get default hugepage size from %s\n",
proc_meminfo);
- return size;
+ if (size == 0) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot get default hugepage size from
%s\n",
+__func__, proc_meminfo);
+ return -1;
+ }
+ *result = size;
+ return 0;
}
static int
@@ -191,11 +198,19 @@
int retval = -1;
FILE *fd = fopen(proc_mounts, "r");
- if (fd == NULL)
- rte_panic("Cannot open %s\n", proc_mounts);
+ if (fd == NULL) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot open %s\n",
+ __func__, proc_mounts);
+ return -ENOENT;
+ }
- if (default_size == 0)
- default_size = get_default_hp_size();
+ if (default_size == 0) {
+ retval = get_default_hp_size(&default_size);
+ if (retval) {
+ fclose(fd);
+ return retval;
+ }
+ }
while (fgets(buf, sizeof(buf), fd)){
if (rte_strsplit(buf, sizeof(buf), splitstr, _FIELDNAME_MAX,
--
1.8.3.1
[dpdk-dev] [PATCH v6 08/11] eal: replace rte_panic instances in interrupts thread
replace panic calls with log and return value.
Thread function removes the noreturn attribute.
Signed-off-by: Arnon Warshavsky
---
lib/librte_eal/linuxapp/eal/eal_interrupts.c | 25 ++---
1 file changed, 18 insertions(+), 7 deletions(-)
diff --git a/lib/librte_eal/linuxapp/eal/eal_interrupts.c
b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
index 58e9328..77e6f2a 100644
--- a/lib/librte_eal/linuxapp/eal/eal_interrupts.c
+++ b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
@@ -785,7 +785,7 @@ struct rte_intr_source {
* @return
* never return;
*/
-static __attribute__((noreturn)) void *
+static void *
eal_intr_thread_main(__rte_unused void *arg)
{
struct epoll_event ev;
@@ -803,8 +803,11 @@ static __attribute__((noreturn)) void *
/* create epoll fd */
int pfd = epoll_create(1);
- if (pfd < 0)
- rte_panic("Cannot create epoll instance\n");
+ if (pfd < 0) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot create epoll
instance\n",
+ __func__);
+ return NULL;
+ }
pipe_event.data.fd = intr_pipe.readfd;
/**
@@ -813,8 +816,11 @@ static __attribute__((noreturn)) void *
*/
if (epoll_ctl(pfd, EPOLL_CTL_ADD, intr_pipe.readfd,
&pipe_event) < 0) {
- rte_panic("Error adding fd to %d epoll_ctl, %s\n",
+ RTE_LOG(CRIT, EAL, "%s(): Error adding fd to %d "
+ "epoll_ctl, %s\n",
+ __func__,
intr_pipe.readfd, strerror(errno));
+ return NULL;
}
numfds++;
@@ -831,9 +837,12 @@ static __attribute__((noreturn)) void *
* into wait list.
*/
if (epoll_ctl(pfd, EPOLL_CTL_ADD,
- src->intr_handle.fd, &ev) < 0){
- rte_panic("Error adding fd %d epoll_ctl, %s\n",
- src->intr_handle.fd, strerror(errno));
+ src->intr_handle.fd, &ev) < 0) {
+ RTE_LOG(CRIT, EAL, "%s(): Error adding fd %d
epoll_ctl, %s\n",
+ __func__,
+ src->intr_handle.fd,
+ strerror(errno));
+ return NULL;
}
else
numfds++;
@@ -848,6 +857,8 @@ static __attribute__((noreturn)) void *
*/
close(pfd);
}
+
+ return NULL;
}
int
--
1.8.3.1
[dpdk-dev] [PATCH v6 10/11] eal: replace rte_panic instances in init sequence
Change some local functions return type from void to int.
This change does not break ABI as the functions are internal.
Panic thrown from threads was not handled in this patch
Signed-off-by: Arnon Warshavsky
---
lib/librte_eal/bsdapp/eal/eal.c | 70
lib/librte_eal/linuxapp/eal/eal.c | 97 ++-
2 files changed, 115 insertions(+), 52 deletions(-)
diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
index d996190..a3c3b37 100644
--- a/lib/librte_eal/bsdapp/eal/eal.c
+++ b/lib/librte_eal/bsdapp/eal/eal.c
@@ -151,7 +151,7 @@ enum rte_iova_mode
* We also don't lock the whole file, so that in future we can use read-locks
* on other parts, e.g. memzones, to detect if there are running secondary
* processes. */
-static void
+static int
rte_eal_config_create(void)
{
void *rte_mem_cfg_addr;
@@ -160,60 +160,81 @@ enum rte_iova_mode
const char *pathname = eal_runtime_config_path();
if (internal_config.no_shconf)
- return;
+ return 0;
if (mem_cfg_fd < 0){
mem_cfg_fd = open(pathname, O_RDWR | O_CREAT, 0660);
- if (mem_cfg_fd < 0)
- rte_panic("Cannot open '%s' for rte_mem_config\n",
pathname);
+ if (mem_cfg_fd < 0) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot open '%s' for
rte_mem_config\n",
+ __func__, pathname);
+ return -1;
+ }
}
retval = ftruncate(mem_cfg_fd, sizeof(*rte_config.mem_config));
if (retval < 0){
close(mem_cfg_fd);
- rte_panic("Cannot resize '%s' for rte_mem_config\n", pathname);
+ mem_cfg_fd = -1;
+ RTE_LOG(CRIT, EAL, "%s(): Cannot resize '%s' for
rte_mem_config\n",
+ __func__, pathname);
+ return -1;
}
retval = fcntl(mem_cfg_fd, F_SETLK, &wr_lock);
if (retval < 0){
close(mem_cfg_fd);
- rte_exit(EXIT_FAILURE, "Cannot create lock on '%s'. Is another
primary "
- "process running?\n", pathname);
+ mem_cfg_fd = -1;
+ RTE_LOG(CRIT, EAL, "%s(): Cannot create lock on '%s'. Is
another primary process running?\n",
+ __func__, pathname);
+ return -1;
}
rte_mem_cfg_addr = mmap(NULL, sizeof(*rte_config.mem_config),
PROT_READ | PROT_WRITE, MAP_SHARED, mem_cfg_fd,
0);
if (rte_mem_cfg_addr == MAP_FAILED){
- rte_panic("Cannot mmap memory for rte_config\n");
+ RTE_LOG(CRIT, EAL, "%s(): Cannot mmap memory for rte_config\n",
+ __func__);
+ return -1;
}
memcpy(rte_mem_cfg_addr, &early_mem_config, sizeof(early_mem_config));
rte_config.mem_config = rte_mem_cfg_addr;
+
+ return 0;
}
/* attach to an existing shared memory config */
-static void
+static int
rte_eal_config_attach(void)
{
void *rte_mem_cfg_addr;
const char *pathname = eal_runtime_config_path();
if (internal_config.no_shconf)
- return;
+ return 0;
if (mem_cfg_fd < 0){
mem_cfg_fd = open(pathname, O_RDWR);
- if (mem_cfg_fd < 0)
- rte_panic("Cannot open '%s' for rte_mem_config\n",
pathname);
+ if (mem_cfg_fd < 0) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot open '%s' for
rte_mem_config\n",
+ __func__, pathname);
+ return -1;
+ }
}
rte_mem_cfg_addr = mmap(NULL, sizeof(*rte_config.mem_config),
PROT_READ | PROT_WRITE, MAP_SHARED, mem_cfg_fd,
0);
close(mem_cfg_fd);
- if (rte_mem_cfg_addr == MAP_FAILED)
- rte_panic("Cannot mmap memory for rte_config\n");
+ if (rte_mem_cfg_addr == MAP_FAILED) {
+ mem_cfg_fd = -1;
+ RTE_LOG(CRIT, EAL, "%s(): Cannot mmap memory for rte_config\n",
+ __func__);
+ return -1;
+ }
rte_config.mem_config = rte_mem_cfg_addr;
+
+ return 0;
}
/* Detect if we are a primary or a secondary process */
@@ -237,23 +258,29 @@ enum rte_proc_type_t
}
/* Sets up rte_config structure with the pointer to shared memory config.*/
-static void
+static int
rte_config_init(void)
{
rte_config.process_type = internal_config.process_type;
Re: [dpdk-dev] [PATCH v6 00/11] al: replace calls to rte_panic and refrain from new instances
please ignore this patchset. The v6 formatting is messed up. resending Sorry for the mess On Tue, Apr 24, 2018 at 9:41 AM, Arnon Warshavsky wrote: > The purpose of this patch series is to cleanup the library code > from paths that end up aborting the process, > and move to checking error values, in order to allow the running process > perform an orderly teardown or other mitigation of the event. > > This patch modifies the majority of rte_panic calls > under lib and drivers, and replaces them with a log message > and an error return code according to context, > that can be propagated up the call stack. > > - Focus was given to the dpdk initialization path > - Some of the panic calls within drivers were left in place where > the call is from within an interrupt or calls that are > on the data path,where there is no simple applicative > route to propagate the error to temination. > These should be handled by the driver maintainers.. > - local void functions with no api were changed to retrun a value > where needed > - No change took place in example and test files > - No change took place for debug assertions calling panic > - A new function was added to devtools/checkpatches.sh > in order to prevent new additions of calls to rte_panic > under lib and drivers. > > Keep calm and don't panic > > --- > > v2: > - reformat error messages so that literal string are in the same line > - fix typo in commit message > - add new return code to doxigen of rte_memzone_free() > > v3: > - submit all 13 patches changed and unchanged in the same patchset > > v4: > - remove 2 patches that are no more relevant > - fix split literal string in error message > - change return value -1 to enum > - split value and success code in a static function > > v5: > - reword commit messages > - revert thread related instances back to panicing > - handle file descriptors with state to reset after eal init failure > in case re initialization takes place > > v6: > - Use pmd log macro rather than rte_log > > > > Arnon Warshavsky (11): > crypto/dpaa: replace rte_panic instances in crypto/dpaa driver > bond: replace rte_panic instances in bonding driver > e1000: replace rte_panic instances in e1000 driver > ixgbe: replace rte_panic instances in ixgbe driver > eal: replace rte_panic instances in eventdev > kni: replace rte_panic instances in kni > eal: replace rte_panic instances in hugepage_info > eal: replace rte_panic instances in interrupts thread > eal: replace rte_panic instances in ethdev > eal: replace rte_panic instances in init sequence > devtools: prevent new instances of rte_panic and rte_exit > > devtools/checkpatches.sh | 95 > +- > drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c | 8 +- > drivers/crypto/dpaa_sec/dpaa_sec.c| 8 +- > drivers/net/bonding/rte_eth_bond_8023ad.c | 29 --- > drivers/net/bonding/rte_eth_bond_8023ad_private.h | 2 +- > drivers/net/bonding/rte_eth_bond_api.c| 22 +++-- > drivers/net/bonding/rte_eth_bond_pmd.c| 9 ++- > drivers/net/bonding/rte_eth_bond_private.h| 2 +- > drivers/net/e1000/e1000_ethdev.h | 2 +- > drivers/net/e1000/igb_ethdev.c| 4 +- > drivers/net/e1000/igb_pf.c| 15 ++-- > drivers/net/ixgbe/ixgbe_ethdev.c | 6 +- > drivers/net/ixgbe/ixgbe_ethdev.h | 2 +- > drivers/net/ixgbe/ixgbe_pf.c | 15 ++-- > lib/librte_eal/bsdapp/eal/eal.c | 70 +++- > lib/librte_eal/linuxapp/eal/eal.c | 97 > +++ > lib/librte_eal/linuxapp/eal/eal_hugepage_info.c | 37 ++--- > lib/librte_eal/linuxapp/eal/eal_interrupts.c | 25 -- > lib/librte_ether/rte_ethdev.c | 42 +++--- > lib/librte_ether/rte_ethdev.h | 4 +- > lib/librte_eventdev/rte_eventdev_pmd_pci.h| 8 +- > lib/librte_eventdev/rte_eventdev_pmd_vdev.h | 8 +- > lib/librte_kni/rte_kni.c | 18 +++-- > lib/librte_kni/rte_kni_fifo.h | 11 ++- > 24 files changed, 396 insertions(+), 143 deletions(-) > > -- > 1.8.3.1 > > -- *Arnon Warshavsky* *Qwilt | work: +972-72-2221634 | mobile: +972-50-8583058 | ar...@qwilt.com *
Re: [dpdk-dev] [PATCH v6 00/11] al: replace calls to rte_panic and refrain from new instances
On Tue, Apr 24, 2018 at 9:44 AM, Arnon Warshavsky wrote: > please ignore this patchset. > The v6 formatting is messed up. resending > Sorry for the mess > My bad. Ignore the ignore request Its gmail thread aggregation view that made me think I mixed v5 and v6. thanks /Arnon
Re: [dpdk-dev] [PATCH v6 01/11] crypto/dpaa: replace rte_panic instances in crypto/dpaa driver
>
>
> > + if (cryptodev->data->dev_private == NULL) {
> > + RTE_LOG(ERR, PMD, "%s() Cannot allocate memzone
> for private device data",
> > + __func__);
>
> dpaa2_sec is already doing private log type, via DPAA2_SEC_LOG macro.
>
> You should go through your patch and make sure there are as few direct
> calls
> to RTE_LOG as possible.
>
Thanks Stephen,
missed that one which seems to be declared but not yet used.Will replace
I may be missing something here.
While seeing the point in a short common form such as PMD_DRV_LOG,
why won't dpaa use the same syntax rather than adding another syntax
variant which does pretty much the same?
Re: [dpdk-dev] [PATCH v6 05/11] eal: replace rte_panic instances in eventdev
Thanks. Same as with dpaa
On Tue, Apr 24, 2018 at 6:06 PM, Stephen Hemminger <
step...@networkplumber.org> wrote:
> On Tue, 24 Apr 2018 09:41:57 +0300
> Arnon Warshavsky wrote:
>
> > + if (eventdev->data->dev_private == NULL) {
> > + RTE_LOG(CRIT, EAL, "%s(): Cannot allocate memzone
> for private device data",
> > + __func__);
> > + return -ENOMEM;
> > + }
>
> In eventdev, use RTE_EDEV_LOG_ERR for this.
>
--
*Arnon Warshavsky*
*Qwilt | work: +972-72-2221634 | mobile: +972-50-8583058 | ar...@qwilt.com
*
[dpdk-dev] [PATCH v7 02/11] bond: replace rte_panic instances in bonding driver
replace panic calls with log and return value.
Local functions to this file,
changing from void to int are non-abi-breaking
Signed-off-by: Arnon Warshavsky
---
drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c | 2 +-
drivers/net/bonding/rte_eth_bond_8023ad.c | 29 ++-
drivers/net/bonding/rte_eth_bond_8023ad_private.h | 2 +-
drivers/net/bonding/rte_eth_bond_api.c| 22 -
drivers/net/bonding/rte_eth_bond_pmd.c| 9 ---
drivers/net/bonding/rte_eth_bond_private.h| 2 +-
6 files changed, 43 insertions(+), 23 deletions(-)
diff --git a/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
b/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
index a78f3a2..a19fb40 100644
--- a/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
+++ b/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
@@ -2884,7 +2884,7 @@ struct rte_security_ops dpaa2_sec_security_ops = {
rte_socket_id());
if (cryptodev->data->dev_private == NULL) {
- DPAA_SEC_ERR("%s() Cannot allocate memzone for private
device data",
+ DPAA2_SEC_ERR("%s() Cannot allocate memzone for private
device data",
__func__);
return -ENOMEM;
}
diff --git a/drivers/net/bonding/rte_eth_bond_8023ad.c
b/drivers/net/bonding/rte_eth_bond_8023ad.c
index c452318..308e623 100644
--- a/drivers/net/bonding/rte_eth_bond_8023ad.c
+++ b/drivers/net/bonding/rte_eth_bond_8023ad.c
@@ -893,7 +893,7 @@
bond_mode_8023ad_periodic_cb, arg);
}
-void
+int
bond_mode_8023ad_activate_slave(struct rte_eth_dev *bond_dev,
uint16_t slave_id)
{
@@ -939,7 +939,7 @@
timer_cancel(&port->warning_timer);
if (port->mbuf_pool != NULL)
- return;
+ return 0;
RTE_ASSERT(port->rx_ring == NULL);
RTE_ASSERT(port->tx_ring == NULL);
@@ -968,8 +968,9 @@
/* Any memory allocation failure in initialization is critical because
* resources can't be free, so reinitialization is impossible. */
if (port->mbuf_pool == NULL) {
- rte_panic("Slave %u: Failed to create memory pool '%s': %s\n",
- slave_id, mem_name, rte_strerror(rte_errno));
+ RTE_BOND_LOG(ERR, "%s() Slave %u: Failed to create memory pool
'%s': %s\n",
+ __func__, slave_id, mem_name, rte_strerror(rte_errno));
+ return -1;
}
snprintf(mem_name, RTE_DIM(mem_name), "slave_%u_rx", slave_id);
@@ -977,8 +978,9 @@
rte_align32pow2(BOND_MODE_8023AX_SLAVE_RX_PKTS),
socket_id, 0);
if (port->rx_ring == NULL) {
- rte_panic("Slave %u: Failed to create rx ring '%s': %s\n",
slave_id,
- mem_name, rte_strerror(rte_errno));
+ RTE_BOND_LOG(ERR, "%s() Slave %u: Failed to create rx ring
'%s': %s\n",
+ __func__, slave_id, mem_name, rte_strerror(rte_errno));
+ return -1;
}
/* TX ring is at least one pkt longer to make room for marker packet. */
@@ -987,9 +989,12 @@
rte_align32pow2(BOND_MODE_8023AX_SLAVE_TX_PKTS + 1),
socket_id, 0);
if (port->tx_ring == NULL) {
- rte_panic("Slave %u: Failed to create tx ring '%s': %s\n",
slave_id,
- mem_name, rte_strerror(rte_errno));
+ RTE_BOND_LOG(ERR, "%s() Slave %u: Fail to create tx ring '%s':
%s\n",
+ __func__, slave_id, mem_name, rte_strerror(rte_errno));
+ return -1;
}
+
+ return 0;
}
int
@@ -1143,9 +1148,11 @@
struct bond_dev_private *internals = bond_dev->data->dev_private;
uint8_t i;
- for (i = 0; i < internals->active_slave_count; i++)
- bond_mode_8023ad_activate_slave(bond_dev,
- internals->active_slaves[i]);
+ for (i = 0; i < internals->active_slave_count; i++) {
+ if (!bond_mode_8023ad_activate_slave(bond_dev,
+ internals->active_slaves[i]))
+ return -1;
+ }
return 0;
}
diff --git a/drivers/net/bonding/rte_eth_bond_8023ad_private.h
b/drivers/net/bonding/rte_eth_bond_8023ad_private.h
index 0f490a5..96a42f2 100644
--- a/drivers/net/bonding/rte_eth_bond_8023ad_private.h
+++ b/drivers/net/bonding/rte_eth_bond_8023ad_private.h
@@ -263,7 +263,7 @@ struct mode8023ad_private {
* @return
* 0 on success, negative value otherwise.
*/
-void
+int
bond_mode_8023ad_activate_slave(struct rte_eth_dev *dev, uint16_t p
[dpdk-dev] [PATCH v7 00/11] eal: replace calls to rte_panic and refrain from new instances
The purpose of this patch series is to cleanup the library code from paths that end up aborting the process, and move to checking error values, in order to allow the running process perform an orderly teardown or other mitigation of the event. This patch modifies the majority of rte_panic calls under lib and drivers, and replaces them with a log message and an error return code according to context, that can be propagated up the call stack. - Focus was given to the dpdk initialization path - Some of the panic calls within drivers were left in place where the call is from within an interrupt or calls that are on the data path,where there is no simple applicative route to propagate the error to temination. These should be handled by the driver maintainers.. - local void functions with no api were changed to retrun a value where needed - No change took place in example and test files - No change took place for debug assertions calling panic - A new function was added to devtools/checkpatches.sh in order to prevent new additions of calls to rte_panic under lib and drivers. Keep calm and don't panic --- v2: - reformat error messages so that literal string are in the same line - fix typo in commit message - add new return code to doxigen of rte_memzone_free() v3: - submit all 13 patches changed and unchanged in the same patchset v4: - remove 2 patches that are no more relevant - fix split literal string in error message - change return value -1 to enum - split value and success code in a static function v5: - reword commit messages - revert thread related instances back to panicing - handle file descriptors with state to reset after eal init failure in case re initialization takes place v6: - Use pmd log macro rather than rte_log v7: - use bond specific , dpaa2 specific and eventdev specific log macros Arnon Warshavsky (11): crypto/dpaa: replace rte_panic instances in crypto/dpaa driver bond: replace rte_panic instances in bonding driver e1000: replace rte_panic instances in e1000 driver ixgbe: replace rte_panic instances in ixgbe driver eal: replace rte_panic instances in eventdev kni: replace rte_panic instances in kni eal: replace rte_panic instances in hugepage_info eal: replace rte_panic instances in interrupts thread eal: replace rte_panic instances in ethdev eal: replace rte_panic instances in init sequence devtools: prevent new instances of rte_panic and rte_exit devtools/checkpatches.sh | 95 +- drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c | 8 +- drivers/crypto/dpaa_sec/dpaa_sec.c| 10 ++- drivers/net/bonding/rte_eth_bond_8023ad.c | 29 --- drivers/net/bonding/rte_eth_bond_8023ad_private.h | 2 +- drivers/net/bonding/rte_eth_bond_api.c| 22 +++-- drivers/net/bonding/rte_eth_bond_pmd.c| 9 ++- drivers/net/bonding/rte_eth_bond_private.h| 2 +- drivers/net/e1000/e1000_ethdev.h | 2 +- drivers/net/e1000/igb_ethdev.c| 4 +- drivers/net/e1000/igb_pf.c| 15 ++-- drivers/net/ixgbe/ixgbe_ethdev.c | 6 +- drivers/net/ixgbe/ixgbe_ethdev.h | 2 +- drivers/net/ixgbe/ixgbe_pf.c | 15 ++-- lib/librte_eal/bsdapp/eal/eal.c | 78 +- lib/librte_eal/linuxapp/eal/eal.c | 97 +++ lib/librte_eal/linuxapp/eal/eal_hugepage_info.c | 37 ++--- lib/librte_eal/linuxapp/eal/eal_interrupts.c | 25 -- lib/librte_ether/rte_ethdev.c | 42 +++--- lib/librte_ether/rte_ethdev.h | 4 +- lib/librte_eventdev/rte_eventdev_pmd_pci.h| 8 +- lib/librte_eventdev/rte_eventdev_pmd_vdev.h | 8 +- lib/librte_kni/rte_kni.c | 18 +++-- lib/librte_kni/rte_kni_fifo.h | 11 ++- 24 files changed, 407 insertions(+), 142 deletions(-) -- 1.8.3.1
[dpdk-dev] [PATCH v7 01/11] crypto/dpaa: replace rte_panic instances in crypto/dpaa driver
replace panic calls with log and return value.
Signed-off-by: Arnon Warshavsky
---
drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c | 8 +---
drivers/crypto/dpaa_sec/dpaa_sec.c | 10 ++
2 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
b/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
index 58cbce8..a78f3a2 100644
--- a/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
+++ b/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
@@ -2883,9 +2883,11 @@ struct rte_security_ops dpaa2_sec_security_ops = {
RTE_CACHE_LINE_SIZE,
rte_socket_id());
- if (cryptodev->data->dev_private == NULL)
- rte_panic("Cannot allocate memzone for private "
- "device data");
+ if (cryptodev->data->dev_private == NULL) {
+ DPAA_SEC_ERR("%s() Cannot allocate memzone for private
device data",
+ __func__);
+ return -ENOMEM;
+ }
}
dpaa2_dev->cryptodev = cryptodev;
diff --git a/drivers/crypto/dpaa_sec/dpaa_sec.c
b/drivers/crypto/dpaa_sec/dpaa_sec.c
index e456fd5..a4670bf 100644
--- a/drivers/crypto/dpaa_sec/dpaa_sec.c
+++ b/drivers/crypto/dpaa_sec/dpaa_sec.c
@@ -2352,7 +2352,7 @@ struct rte_security_ops dpaa_sec_security_ops = {
}
}
- RTE_LOG(INFO, PMD, "%s cryptodev init\n", cryptodev->data->name);
+ DPAA_SEC_INFO("%s cryptodev init\n", cryptodev->data->name);
return 0;
init_error:
@@ -2384,9 +2384,11 @@ struct rte_security_ops dpaa_sec_security_ops = {
RTE_CACHE_LINE_SIZE,
rte_socket_id());
- if (cryptodev->data->dev_private == NULL)
- rte_panic("Cannot allocate memzone for private "
- "device data");
+ if (cryptodev->data->dev_private == NULL) {
+ DPAA_SEC_ERR("%s() Cannot allocate memzone for private
device data",
+ __func__);
+ return -ENOMEM;
+ }
}
dpaa_dev->crypto_dev = cryptodev;
--
1.8.3.1
[dpdk-dev] [PATCH v7 03/11] e1000: replace rte_panic instances in e1000 driver
replace panic calls with log and return value.
Local function to this file,
changing from void to int is non-abi-breaking
Signed-off-by: Arnon Warshavsky
---
drivers/net/e1000/e1000_ethdev.h | 2 +-
drivers/net/e1000/igb_ethdev.c | 4 +++-
drivers/net/e1000/igb_pf.c | 15 +--
3 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/drivers/net/e1000/e1000_ethdev.h b/drivers/net/e1000/e1000_ethdev.h
index 6354b89..2e527de 100644
--- a/drivers/net/e1000/e1000_ethdev.h
+++ b/drivers/net/e1000/e1000_ethdev.h
@@ -411,7 +411,7 @@ int eth_igb_rss_hash_conf_get(struct rte_eth_dev *dev,
/*
* misc function prototypes
*/
-void igb_pf_host_init(struct rte_eth_dev *eth_dev);
+int igb_pf_host_init(struct rte_eth_dev *eth_dev);
void igb_pf_mbx_process(struct rte_eth_dev *eth_dev);
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index 9b808a9..67a32a2 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -833,7 +833,9 @@ static int igb_flex_filter_uninit(struct rte_eth_dev
*eth_dev)
}
/* initialize PF if max_vfs not zero */
- igb_pf_host_init(eth_dev);
+ error = igb_pf_host_init(eth_dev);
+ if (error != 0)
+ goto err_late;
ctrl_ext = E1000_READ_REG(hw, E1000_CTRL_EXT);
/* Set PF Reset Done bit so PF/VF Mail Ops can work */
diff --git a/drivers/net/e1000/igb_pf.c b/drivers/net/e1000/igb_pf.c
index b9f2e53..6e511a9 100644
--- a/drivers/net/e1000/igb_pf.c
+++ b/drivers/net/e1000/igb_pf.c
@@ -63,7 +63,7 @@ int igb_vf_perm_addr_gen(struct rte_eth_dev *dev, uint16_t
vf_num)
return 0;
}
-void igb_pf_host_init(struct rte_eth_dev *eth_dev)
+int igb_pf_host_init(struct rte_eth_dev *eth_dev)
{
struct e1000_vf_info **vfinfo =
E1000_DEV_PRIVATE_TO_P_VFDATA(eth_dev->data->dev_private);
@@ -74,7 +74,7 @@ void igb_pf_host_init(struct rte_eth_dev *eth_dev)
RTE_ETH_DEV_SRIOV(eth_dev).active = 0;
if (0 == (vf_num = dev_num_vf(eth_dev)))
- return;
+ return 0;
if (hw->mac.type == e1000_i350)
nb_queue = 1;
@@ -82,11 +82,14 @@ void igb_pf_host_init(struct rte_eth_dev *eth_dev)
/* per datasheet, it should be 2, but 1 seems correct */
nb_queue = 1;
else
- return;
+ return 0;
*vfinfo = rte_zmalloc("vf_info", sizeof(struct e1000_vf_info) * vf_num,
0);
- if (*vfinfo == NULL)
- rte_panic("Cannot allocate memory for private VF data\n");
+ if (*vfinfo == NULL) {
+ PMD_DRV_LOG(CRIT, "%s(): Cannot allocate memory for private VF
data\n",
+ __func__);
+ return -ENOMEM;
+ }
RTE_ETH_DEV_SRIOV(eth_dev).active = ETH_8_POOLS;
RTE_ETH_DEV_SRIOV(eth_dev).nb_q_per_pool = nb_queue;
@@ -98,7 +101,7 @@ void igb_pf_host_init(struct rte_eth_dev *eth_dev)
/* set mb interrupt mask */
igb_mb_intr_setup(eth_dev);
- return;
+ return 0;
}
void igb_pf_host_uninit(struct rte_eth_dev *dev)
--
1.8.3.1
[dpdk-dev] [PATCH v7 04/11] ixgbe: replace rte_panic instances in ixgbe driver
replace panic calls with log and return value.
Local function to this file,
changing from void to int is non-abi-breaking
Signed-off-by: Arnon Warshavsky
---
drivers/net/ixgbe/ixgbe_ethdev.c | 6 --
drivers/net/ixgbe/ixgbe_ethdev.h | 2 +-
drivers/net/ixgbe/ixgbe_pf.c | 15 ++-
3 files changed, 15 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index a5e2fc0..fb95cc7 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -1061,7 +1061,7 @@ struct rte_ixgbe_xstats_name_off {
IXGBE_DEV_PRIVATE_TO_BW_CONF(eth_dev->data->dev_private);
uint32_t ctrl_ext;
uint16_t csum;
- int diag, i;
+ int diag, i, error;
PMD_INIT_FUNC_TRACE();
@@ -1224,7 +1224,9 @@ struct rte_ixgbe_xstats_name_off {
memset(hwstrip, 0, sizeof(*hwstrip));
/* initialize PF if max_vfs not zero */
- ixgbe_pf_host_init(eth_dev);
+ error = ixgbe_pf_host_init(eth_dev);
+ if (error != 0)
+ return error;
ctrl_ext = IXGBE_READ_REG(hw, IXGBE_CTRL_EXT);
/* let hardware know driver is loaded */
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
index 6550777..8bb41ec 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.h
+++ b/drivers/net/ixgbe/ixgbe_ethdev.h
@@ -661,7 +661,7 @@ int ixgbe_fdir_filter_program(struct rte_eth_dev *dev,
void ixgbe_vlan_hw_strip_config(struct rte_eth_dev *dev);
-void ixgbe_pf_host_init(struct rte_eth_dev *eth_dev);
+int ixgbe_pf_host_init(struct rte_eth_dev *eth_dev);
void ixgbe_pf_host_uninit(struct rte_eth_dev *eth_dev);
diff --git a/drivers/net/ixgbe/ixgbe_pf.c b/drivers/net/ixgbe/ixgbe_pf.c
index 4e61310..81a9910 100644
--- a/drivers/net/ixgbe/ixgbe_pf.c
+++ b/drivers/net/ixgbe/ixgbe_pf.c
@@ -66,7 +66,7 @@ int ixgbe_vf_perm_addr_gen(struct rte_eth_dev *dev, uint16_t
vf_num)
return 0;
}
-void ixgbe_pf_host_init(struct rte_eth_dev *eth_dev)
+int ixgbe_pf_host_init(struct rte_eth_dev *eth_dev)
{
struct ixgbe_vf_info **vfinfo =
IXGBE_DEV_PRIVATE_TO_P_VFDATA(eth_dev->data->dev_private);
@@ -84,11 +84,14 @@ void ixgbe_pf_host_init(struct rte_eth_dev *eth_dev)
RTE_ETH_DEV_SRIOV(eth_dev).active = 0;
vf_num = dev_num_vf(eth_dev);
if (vf_num == 0)
- return;
+ return 0;
*vfinfo = rte_zmalloc("vf_info", sizeof(struct ixgbe_vf_info) * vf_num,
0);
- if (*vfinfo == NULL)
- rte_panic("Cannot allocate memory for private VF data\n");
+ if (*vfinfo == NULL) {
+ PMD_DRV_LOG(ERR, "%s() Cannot allocate memory for private VF
data\n",
+ __func__);
+ return -ENOMEM;
+ }
memset(mirror_info, 0, sizeof(struct ixgbe_mirror_info));
memset(uta_info, 0, sizeof(struct ixgbe_uta_info));
@@ -116,6 +119,8 @@ void ixgbe_pf_host_init(struct rte_eth_dev *eth_dev)
/* set mb interrupt mask */
ixgbe_mb_intr_setup(eth_dev);
+
+ return 0;
}
void ixgbe_pf_host_uninit(struct rte_eth_dev *eth_dev)
@@ -203,7 +208,7 @@ int ixgbe_pf_host_configure(struct rte_eth_dev *eth_dev)
vf_num = dev_num_vf(eth_dev);
if (vf_num == 0)
- return -1;
+ return -ENOMEM;
/* enable VMDq and set the default pool for PF */
vtctl = IXGBE_READ_REG(hw, IXGBE_VT_CTL);
--
1.8.3.1
[dpdk-dev] [PATCH v7 05/11] eal: replace rte_panic instances in eventdev
replace panic calls with log and return value.
Signed-off-by: Arnon Warshavsky
---
lib/librte_eventdev/rte_eventdev_pmd_pci.h | 8 +---
lib/librte_eventdev/rte_eventdev_pmd_vdev.h | 8 +---
2 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/lib/librte_eventdev/rte_eventdev_pmd_pci.h
b/lib/librte_eventdev/rte_eventdev_pmd_pci.h
index 8fb6138..d4d10c8 100644
--- a/lib/librte_eventdev/rte_eventdev_pmd_pci.h
+++ b/lib/librte_eventdev/rte_eventdev_pmd_pci.h
@@ -66,9 +66,11 @@
RTE_CACHE_LINE_SIZE,
rte_socket_id());
- if (eventdev->data->dev_private == NULL)
- rte_panic("Cannot allocate memzone for private "
- "device data");
+ if (eventdev->data->dev_private == NULL) {
+ RTE_EDEV_LOG_ERR("%s(): Cannot allocate memzone for
private device data",
+ __func__);
+ return -ENOMEM;
+ }
}
eventdev->dev = &pci_dev->device;
diff --git a/lib/librte_eventdev/rte_eventdev_pmd_vdev.h
b/lib/librte_eventdev/rte_eventdev_pmd_vdev.h
index 8c64a06..3c35aac 100644
--- a/lib/librte_eventdev/rte_eventdev_pmd_vdev.h
+++ b/lib/librte_eventdev/rte_eventdev_pmd_vdev.h
@@ -61,9 +61,11 @@
RTE_CACHE_LINE_SIZE,
socket_id);
- if (eventdev->data->dev_private == NULL)
- rte_panic("Cannot allocate memzone for private device"
- " data");
+ if (eventdev->data->dev_private == NULL) {
+ RTE_EDEV_LOG_ERR("%s(): Cannot allocate memzone for
private device data",
+ __func__);
+ return NULL;
+ }
}
return eventdev;
--
1.8.3.1
[dpdk-dev] [PATCH v7 06/11] kni: replace rte_panic instances in kni
replace panic calls with log and return value.
Local function to this file,
changing from void to int is non-abi-breaking
Signed-off-by: Arnon Warshavsky
---
lib/librte_kni/rte_kni.c | 18 --
lib/librte_kni/rte_kni_fifo.h | 11 ---
2 files changed, 20 insertions(+), 9 deletions(-)
diff --git a/lib/librte_kni/rte_kni.c b/lib/librte_kni/rte_kni.c
index 8a8f6c1..4dac407 100644
--- a/lib/librte_kni/rte_kni.c
+++ b/lib/librte_kni/rte_kni.c
@@ -353,37 +353,43 @@ struct rte_kni *
/* TX RING */
mz = slot->m_tx_q;
ctx->tx_q = mz->addr;
- kni_fifo_init(ctx->tx_q, KNI_FIFO_COUNT_MAX);
+ if (kni_fifo_init(ctx->tx_q, KNI_FIFO_COUNT_MAX))
+ return NULL;
dev_info.tx_phys = mz->phys_addr;
/* RX RING */
mz = slot->m_rx_q;
ctx->rx_q = mz->addr;
- kni_fifo_init(ctx->rx_q, KNI_FIFO_COUNT_MAX);
+ if (kni_fifo_init(ctx->rx_q, KNI_FIFO_COUNT_MAX))
+ return NULL;
dev_info.rx_phys = mz->phys_addr;
/* ALLOC RING */
mz = slot->m_alloc_q;
ctx->alloc_q = mz->addr;
- kni_fifo_init(ctx->alloc_q, KNI_FIFO_COUNT_MAX);
+ if (kni_fifo_init(ctx->alloc_q, KNI_FIFO_COUNT_MAX))
+ return NULL;
dev_info.alloc_phys = mz->phys_addr;
/* FREE RING */
mz = slot->m_free_q;
ctx->free_q = mz->addr;
- kni_fifo_init(ctx->free_q, KNI_FIFO_COUNT_MAX);
+ if (kni_fifo_init(ctx->free_q, KNI_FIFO_COUNT_MAX))
+ return NULL;
dev_info.free_phys = mz->phys_addr;
/* Request RING */
mz = slot->m_req_q;
ctx->req_q = mz->addr;
- kni_fifo_init(ctx->req_q, KNI_FIFO_COUNT_MAX);
+ if (kni_fifo_init(ctx->req_q, KNI_FIFO_COUNT_MAX))
+ return NULL;
dev_info.req_phys = mz->phys_addr;
/* Response RING */
mz = slot->m_resp_q;
ctx->resp_q = mz->addr;
- kni_fifo_init(ctx->resp_q, KNI_FIFO_COUNT_MAX);
+ if (kni_fifo_init(ctx->resp_q, KNI_FIFO_COUNT_MAX))
+ return NULL;
dev_info.resp_phys = mz->phys_addr;
/* Req/Resp sync mem area */
diff --git a/lib/librte_kni/rte_kni_fifo.h b/lib/librte_kni/rte_kni_fifo.h
index ac26a8c..5052015 100644
--- a/lib/librte_kni/rte_kni_fifo.h
+++ b/lib/librte_kni/rte_kni_fifo.h
@@ -7,17 +7,22 @@
/**
* Initializes the kni fifo structure
*/
-static void
+static int
kni_fifo_init(struct rte_kni_fifo *fifo, unsigned size)
{
/* Ensure size is power of 2 */
- if (size & (size - 1))
- rte_panic("KNI fifo size must be power of 2\n");
+ if (size & (size - 1)) {
+ RTE_LOG(CRIT, EAL, "%s(): KNI fifo size must be power of 2\n",
+ __func__);
+ return -1;
+ }
fifo->write = 0;
fifo->read = 0;
fifo->len = size;
fifo->elem_size = sizeof(void *);
+
+ return 0;
}
/**
--
1.8.3.1
[dpdk-dev] [PATCH v7 07/11] eal: replace rte_panic instances in hugepage_info
replace panic calls with log and return value.
Signed-off-by: Arnon Warshavsky
---
lib/librte_eal/linuxapp/eal/eal_hugepage_info.c | 37 +
1 file changed, 26 insertions(+), 11 deletions(-)
diff --git a/lib/librte_eal/linuxapp/eal/eal_hugepage_info.c
b/lib/librte_eal/linuxapp/eal/eal_hugepage_info.c
index db5aabd..797b8fa 100644
--- a/lib/librte_eal/linuxapp/eal/eal_hugepage_info.c
+++ b/lib/librte_eal/linuxapp/eal/eal_hugepage_info.c
@@ -145,8 +145,8 @@
return num_pages;
}
-static uint64_t
-get_default_hp_size(void)
+static int
+get_default_hp_size(uint64_t *result)
{
const char proc_meminfo[] = "/proc/meminfo";
const char str_hugepagesz[] = "Hugepagesize:";
@@ -155,8 +155,11 @@
unsigned long long size = 0;
FILE *fd = fopen(proc_meminfo, "r");
- if (fd == NULL)
- rte_panic("Cannot open %s\n", proc_meminfo);
+ if (fd == NULL) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot open %s\n",
+ __func__, proc_meminfo);
+ return -1;
+ }
while(fgets(buffer, sizeof(buffer), fd)){
if (strncmp(buffer, str_hugepagesz, hugepagesz_len) == 0){
size = rte_str_to_size(&buffer[hugepagesz_len]);
@@ -164,9 +167,13 @@
}
}
fclose(fd);
- if (size == 0)
- rte_panic("Cannot get default hugepage size from %s\n",
proc_meminfo);
- return size;
+ if (size == 0) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot get default hugepage size from
%s\n",
+__func__, proc_meminfo);
+ return -1;
+ }
+ *result = size;
+ return 0;
}
static int
@@ -191,11 +198,19 @@
int retval = -1;
FILE *fd = fopen(proc_mounts, "r");
- if (fd == NULL)
- rte_panic("Cannot open %s\n", proc_mounts);
+ if (fd == NULL) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot open %s\n",
+ __func__, proc_mounts);
+ return -ENOENT;
+ }
- if (default_size == 0)
- default_size = get_default_hp_size();
+ if (default_size == 0) {
+ retval = get_default_hp_size(&default_size);
+ if (retval) {
+ fclose(fd);
+ return retval;
+ }
+ }
while (fgets(buf, sizeof(buf), fd)){
if (rte_strsplit(buf, sizeof(buf), splitstr, _FIELDNAME_MAX,
--
1.8.3.1
[dpdk-dev] [PATCH v7 09/11] eal: replace rte_panic instances in ethdev
Local function to this file,
changing from void to int is non-abi-breaking
Signed-off-by: Arnon Warshavsky
---
lib/librte_ether/rte_ethdev.c | 42 ++
lib/librte_ether/rte_ethdev.h | 4 +++-
2 files changed, 33 insertions(+), 13 deletions(-)
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 7821a88..4ffdc54 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -194,7 +194,7 @@ enum {
return port_id;
}
-static void
+static int
rte_eth_dev_shared_data_prepare(void)
{
const unsigned flags = 0;
@@ -210,8 +210,12 @@ enum {
rte_socket_id(), flags);
} else
mz = rte_memzone_lookup(MZ_RTE_ETH_DEV_DATA);
- if (mz == NULL)
- rte_panic("Cannot allocate ethdev shared data\n");
+ if (mz == NULL) {
+ rte_spinlock_unlock(&rte_eth_shared_data_lock);
+ RTE_LOG(CRIT, EAL, "%s(): Cannot allocate ethdev shared
data\n",
+ __func__);
+ return -1;
+ }
rte_eth_dev_shared_data = mz->addr;
if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
@@ -224,6 +228,8 @@ enum {
}
rte_spinlock_unlock(&rte_eth_shared_data_lock);
+
+ return 0;
}
struct rte_eth_dev *
@@ -274,7 +280,8 @@ struct rte_eth_dev *
uint16_t port_id;
struct rte_eth_dev *eth_dev = NULL;
- rte_eth_dev_shared_data_prepare();
+ if (rte_eth_dev_shared_data_prepare() != 0)
+ return NULL;
/* Synchronize port creation between primary and secondary threads. */
rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
@@ -317,7 +324,8 @@ struct rte_eth_dev *
uint16_t i;
struct rte_eth_dev *eth_dev = NULL;
- rte_eth_dev_shared_data_prepare();
+ if (rte_eth_dev_shared_data_prepare() != 0)
+ return NULL;
/* Synchronize port attachment to primary port creation and release. */
rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
@@ -345,7 +353,8 @@ struct rte_eth_dev *
if (eth_dev == NULL)
return -EINVAL;
- rte_eth_dev_shared_data_prepare();
+ if (rte_eth_dev_shared_data_prepare() != 0)
+ return -1;
rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
@@ -399,7 +408,8 @@ struct rte_eth_dev *
int __rte_experimental
rte_eth_dev_owner_new(uint64_t *owner_id)
{
- rte_eth_dev_shared_data_prepare();
+ if (rte_eth_dev_shared_data_prepare() != 0)
+ return -1;
rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
@@ -450,7 +460,8 @@ struct rte_eth_dev *
{
int ret;
- rte_eth_dev_shared_data_prepare();
+ if (rte_eth_dev_shared_data_prepare() != 0)
+ return -1;
rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
@@ -467,7 +478,8 @@ struct rte_eth_dev *
{.id = RTE_ETH_DEV_NO_OWNER, .name = ""};
int ret;
- rte_eth_dev_shared_data_prepare();
+ if (rte_eth_dev_shared_data_prepare() != 0)
+ return -1;
rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
@@ -477,12 +489,15 @@ struct rte_eth_dev *
return ret;
}
-void __rte_experimental
+int __rte_experimental
rte_eth_dev_owner_delete(const uint64_t owner_id)
{
uint16_t port_id;
+ int error;
- rte_eth_dev_shared_data_prepare();
+ error = rte_eth_dev_shared_data_prepare();
+ if (error != 0)
+ return error;
rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
@@ -495,6 +510,8 @@ struct rte_eth_dev *
}
rte_spinlock_unlock(&rte_eth_dev_shared_data->ownership_lock);
+
+ return 0;
}
int __rte_experimental
@@ -502,7 +519,8 @@ struct rte_eth_dev *
{
int ret = 0;
- rte_eth_dev_shared_data_prepare();
+ if (rte_eth_dev_shared_data_prepare() != 0)
+ return -1;
rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock);
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index b9eb8ae..46e5947 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -1354,8 +1354,10 @@ int __rte_experimental rte_eth_dev_owner_unset(const
uint16_t port_id,
*
* @param owner_id
* The owner identifier.
+ * @return
+ * 0 on success, negative errno value on error.
*/
-void __rte_experimental rte_eth_dev_owner_delete(const uint64_t owner_id);
+int __rte_experimental rte_eth_dev_owner_delete(const uint64_t owner_id);
/**
* @warning
--
1.8.3.1
[dpdk-dev] [PATCH v7 08/11] eal: replace rte_panic instances in interrupts thread
replace panic calls with log and return value.
Thread function removes the noreturn attribute.
Signed-off-by: Arnon Warshavsky
---
lib/librte_eal/linuxapp/eal/eal_interrupts.c | 25 ++---
1 file changed, 18 insertions(+), 7 deletions(-)
diff --git a/lib/librte_eal/linuxapp/eal/eal_interrupts.c
b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
index 58e9328..77e6f2a 100644
--- a/lib/librte_eal/linuxapp/eal/eal_interrupts.c
+++ b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
@@ -785,7 +785,7 @@ struct rte_intr_source {
* @return
* never return;
*/
-static __attribute__((noreturn)) void *
+static void *
eal_intr_thread_main(__rte_unused void *arg)
{
struct epoll_event ev;
@@ -803,8 +803,11 @@ static __attribute__((noreturn)) void *
/* create epoll fd */
int pfd = epoll_create(1);
- if (pfd < 0)
- rte_panic("Cannot create epoll instance\n");
+ if (pfd < 0) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot create epoll
instance\n",
+ __func__);
+ return NULL;
+ }
pipe_event.data.fd = intr_pipe.readfd;
/**
@@ -813,8 +816,11 @@ static __attribute__((noreturn)) void *
*/
if (epoll_ctl(pfd, EPOLL_CTL_ADD, intr_pipe.readfd,
&pipe_event) < 0) {
- rte_panic("Error adding fd to %d epoll_ctl, %s\n",
+ RTE_LOG(CRIT, EAL, "%s(): Error adding fd to %d "
+ "epoll_ctl, %s\n",
+ __func__,
intr_pipe.readfd, strerror(errno));
+ return NULL;
}
numfds++;
@@ -831,9 +837,12 @@ static __attribute__((noreturn)) void *
* into wait list.
*/
if (epoll_ctl(pfd, EPOLL_CTL_ADD,
- src->intr_handle.fd, &ev) < 0){
- rte_panic("Error adding fd %d epoll_ctl, %s\n",
- src->intr_handle.fd, strerror(errno));
+ src->intr_handle.fd, &ev) < 0) {
+ RTE_LOG(CRIT, EAL, "%s(): Error adding fd %d
epoll_ctl, %s\n",
+ __func__,
+ src->intr_handle.fd,
+ strerror(errno));
+ return NULL;
}
else
numfds++;
@@ -848,6 +857,8 @@ static __attribute__((noreturn)) void *
*/
close(pfd);
}
+
+ return NULL;
}
int
--
1.8.3.1
[dpdk-dev] [PATCH v7 10/11] eal: replace rte_panic instances in init sequence
Change some local functions return type from void to int.
This change does not break ABI as the functions are internal.
Panic thrown from threads was not handled in this patch
Signed-off-by: Arnon Warshavsky
---
lib/librte_eal/bsdapp/eal/eal.c | 78 +++
lib/librte_eal/linuxapp/eal/eal.c | 97 ++-
2 files changed, 125 insertions(+), 50 deletions(-)
diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
index d315cde..bdda8c1 100644
--- a/lib/librte_eal/bsdapp/eal/eal.c
+++ b/lib/librte_eal/bsdapp/eal/eal.c
@@ -151,7 +151,7 @@ enum rte_iova_mode
* We also don't lock the whole file, so that in future we can use read-locks
* on other parts, e.g. memzones, to detect if there are running secondary
* processes. */
-static void
+static int
rte_eal_config_create(void)
{
void *rte_mem_cfg_addr;
@@ -160,60 +160,81 @@ enum rte_iova_mode
const char *pathname = eal_runtime_config_path();
if (internal_config.no_shconf)
- return;
+ return 0;
if (mem_cfg_fd < 0){
mem_cfg_fd = open(pathname, O_RDWR | O_CREAT, 0660);
- if (mem_cfg_fd < 0)
- rte_panic("Cannot open '%s' for rte_mem_config\n",
pathname);
+ if (mem_cfg_fd < 0) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot open '%s' for
rte_mem_config\n",
+ __func__, pathname);
+ return -1;
+ }
}
retval = ftruncate(mem_cfg_fd, sizeof(*rte_config.mem_config));
if (retval < 0){
close(mem_cfg_fd);
- rte_panic("Cannot resize '%s' for rte_mem_config\n", pathname);
+ mem_cfg_fd = -1;
+ RTE_LOG(CRIT, EAL, "%s(): Cannot resize '%s' for
rte_mem_config\n",
+ __func__, pathname);
+ return -1;
}
retval = fcntl(mem_cfg_fd, F_SETLK, &wr_lock);
if (retval < 0){
close(mem_cfg_fd);
- rte_exit(EXIT_FAILURE, "Cannot create lock on '%s'. Is another
primary "
- "process running?\n", pathname);
+ mem_cfg_fd = -1;
+ RTE_LOG(CRIT, EAL, "%s(): Cannot create lock on '%s'. Is
another primary process running?\n",
+ __func__, pathname);
+ return -1;
}
rte_mem_cfg_addr = mmap(NULL, sizeof(*rte_config.mem_config),
PROT_READ | PROT_WRITE, MAP_SHARED, mem_cfg_fd,
0);
if (rte_mem_cfg_addr == MAP_FAILED){
- rte_panic("Cannot mmap memory for rte_config\n");
+ RTE_LOG(CRIT, EAL, "%s(): Cannot mmap memory for rte_config\n",
+ __func__);
+ return -1;
}
memcpy(rte_mem_cfg_addr, &early_mem_config, sizeof(early_mem_config));
rte_config.mem_config = rte_mem_cfg_addr;
+
+ return 0;
}
/* attach to an existing shared memory config */
-static void
+static int
rte_eal_config_attach(void)
{
void *rte_mem_cfg_addr;
const char *pathname = eal_runtime_config_path();
if (internal_config.no_shconf)
- return;
+ return 0;
if (mem_cfg_fd < 0){
mem_cfg_fd = open(pathname, O_RDWR);
- if (mem_cfg_fd < 0)
- rte_panic("Cannot open '%s' for rte_mem_config\n",
pathname);
+ if (mem_cfg_fd < 0) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot open '%s' for
rte_mem_config\n",
+ __func__, pathname);
+ return -1;
+ }
}
rte_mem_cfg_addr = mmap(NULL, sizeof(*rte_config.mem_config),
PROT_READ | PROT_WRITE, MAP_SHARED, mem_cfg_fd,
0);
close(mem_cfg_fd);
- if (rte_mem_cfg_addr == MAP_FAILED)
- rte_panic("Cannot mmap memory for rte_config\n");
+ if (rte_mem_cfg_addr == MAP_FAILED) {
+ mem_cfg_fd = -1;
+ RTE_LOG(CRIT, EAL, "%s(): Cannot mmap memory for rte_config\n",
+ __func__);
+ return -1;
+ }
rte_config.mem_config = rte_mem_cfg_addr;
+
+ return 0;
}
/* Detect if we are a primary or a secondary process */
@@ -237,23 +258,29 @@ enum rte_proc_type_t
}
/* Sets up rte_config structure with the pointer to shared memory config.*/
-static void
+static int
rte_config_init(void)
{
rte_config.process_type = internal_config.process_type;
[dpdk-dev] [PATCH v7 11/11] devtools: prevent new instances of rte_panic and rte_exit
This patch adds a new function that is called
per every checked patch,
and alerts for new instances of rte_panic/rte_exit.
The check excludes comments, and alerts in the case
of a positive balance between additions and removals.
Signed-off-by: Arnon Warshavsky
---
devtools/checkpatches.sh | 95 +++-
1 file changed, 94 insertions(+), 1 deletion(-)
diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
index 7676a6b..48b2685 100755
--- a/devtools/checkpatches.sh
+++ b/devtools/checkpatches.sh
@@ -61,6 +61,91 @@ print_usage () {
END_OF_HELP
}
+check_forbidden_additions() { #
+# -
+#This awk script receives a list of expressions to monitor
+#and a list of folders to search these expressions in
+# - No search is done inside comments
+# - Both additions and removals of the expressions are checked
+# A positive balance of additions fails the check
+ read -d '' awk_script << 'EOF'
+ BEGIN{
+ split(FOLDERS,deny_folders," ");
+ split(EXPRESSIONS,deny_expr," ");
+ in_file=0;
+ in_comment=0;
+ count=0;
+ comment_start="/*"
+ comment_end="*/"
+}
+# search for add/remove instances in current file
+# state machine assumes the comments structure is enforced by
+# checkpatches.pl
+(in_file) {
+ # comment start
+ if (index($0,comment_start) > 0){
+ in_comment = 1
+ }
+ # non comment code
+ if (in_comment == 0) {
+ for (i in deny_expr) {
+ forbidden_added = "^\+.*" deny_expr[i];
+ forbidden_removed="^-.*" deny_expr[i];
+ current = expressions[deny_expr[i]]
+ if ($0 ~ forbidden_added) {
+ count = count + 1;
+ expressions[deny_expr[i]] = current + 1
+ }
+ if ($0 ~ forbidden_removed) {
+ count = count - 1;
+ expressions[deny_expr[i]] = current - 1
+ }
+ }
+ }
+
+ # comment end
+ if (index($0,comment_end) > 0) {
+ in_comment = 0
+ }
+}
+ # switch to next file , check if the balance of add/remove
+ # of previous filehad new additions
+ ($0 ~ "^\+\+\+ b/") {
+ in_file = 0;
+ if (count > 0){
+ exit;
+ }
+ for (i in deny_folders){
+ re = "^\+\+\+ b/" deny_folders[i];
+ if ($0 ~ deny_folders[i]) {
+ in_file = 1
+ last_file = $0
+ }
+ }
+ }
+ END{
+ if (count > 0){
+ print "Warning: in " substr(last_file,6) ":"
+ print "are you sure you want to add the following:"
+ for (key in expressions) {
+ if (expressions[key] > 0) {
+ print key
+ }
+ }
+ exit 1
+ }
+ }
+EOF
+ # -
+
+ # refrain from new additions of rte_panic() and rte_exit()
+ # under lib and net
+ # multiple folders and expressions are separated by spaces
+ awk -v FOLDERS="lib net" \
+ -v EXPRESSIONS="rte_panic\\\( rte_exit\\\(" \
+ "$awk_script" -
+}
+
number=0
quiet=false
verbose=false
@@ -89,11 +174,19 @@ check () { #
total=$(($total + 1))
! $verbose || printf '\n### %s\n\n' "$3"
if [ -n "$1" ] ; then
+ cat "$1" | check_forbidden_additions
+ [ $? -eq 0 ] || return 0
report=$($DPDK_CHECKPATCH_PATH $options "$1" 2>/dev/null)
elif [ -n "$2" ] ; then
- report=$(git format-patch --find-renames --no-stat --stdout -1
$commit |
+ params=$(echo "--find-renames --no-stat --stdout -1")
+ body=$(git format-patch $params $commit)
+ echo "$body" | check_forbidden_additions
+ [ $? -eq 0 ] || return 0
+ report=$(echo "$body" |
$DPDK_CHECKPATCH_PATH $options - 2>/dev/null)
else
+ check_forbidden_additions -
+ [ $? -eq 0 ] || return 0
report=$($DPDK_CHECKPATCH_PATH $options - 2>/dev/null)
fi
[ $? -ne 0 ] || return 0
--
1.8.3.1
Re: [dpdk-dev] [PATCH v7 10/11] eal: replace rte_panic instances in init sequence
<...>
>
> + if (rte_config_init() != 0) {
>> + rte_eal_init_alert("Failed to init configuration");
>> + rte_errno = EFAULT;
>> + return -1;
>> + }
>> +
>> + if (rte_mp_channel_init() < 0) {
>> + rte_eal_init_alert("failed to init mp channel\n");
>> + if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
>> + rte_errno = EFAULT;
>> + return -1;
>> + }
>> + }
>>
>
> ^^^ this change looks unintended. Rebase artifact?
>
> +
>> /* in secondary processes, memory init may allocate additional
>> fbarrays
>> * not present in primary processes, so to avoid any potential
>> issues,
>> * initialize memzones first.
>> @@ -671,6 +712,7 @@ static void rte_eal_init_alert(const char *msg)
>> */
>> if (pipe(lcore_config[i].pipe_master2slave) < 0)
>> rte_panic("Cannot create pipe\n");
>> +
>> if (pipe(lcore_config[i].pipe_slave2master) < 0)
>> rte_panic("Cannot create pipe\n");
>>
>
> ^^^ this looks unintended as well.
>
> diff --git a/lib/librte_eal/linuxapp/eal/eal.c
>> b/lib/librte_eal/linuxapp/eal/eal.c
>> index 5b23bf0..54adaec 100644
>> --- a/lib/librte_eal/linuxapp/eal/eal.c
>> +++ b/lib/librte_eal/linuxapp/eal/eal.c
>> @@ -160,7 +160,7 @@ enum rte_iova_mode
>>* We also don't lock the whole file, so that in future we can use
>> read-locks
>>* on other parts, e.g. memzones, to detect if there are running
>> secondary
>>* processes. */
>> -static void
>> +static int
>> rte_eal_config_create(void)
>> {
>> void *rte_mem_cfg_addr;
>> @@ -169,7 +169,7 @@ enum rte_iova_mode
>> const char *pathname = eal_runtime_config_path();
>> if (internal_config.no_shconf)
>> - return;
>>
>
> <...>
>
> }
>> rte_mem_cfg_addr = mmap(rte_mem_cfg_addr,
>> sizeof(*rte_config.mem_config),
>> PROT_READ | PROT_WRITE, MAP_SHARED,
>> mem_cfg_fd, 0);
>> - if (rte_mem_cfg_addr == MAP_FAILED){
>> - rte_panic("Cannot mmap memory for rte_config\n");
>> + if (rte_mem_cfg_addr == MAP_FAILED) {
>> + RTE_LOG(CRIT, EAL, "%s(): Cannot mmap memory for
>> rte_config\n",
>> + __func__);
>> + return -1;
>> }
>>
>
> I think you forgot to close mem_cfg_fd and set it to -1 in case of error
> here.
>
> memcpy(rte_mem_cfg_addr, &early_mem_config,
>> sizeof(early_mem_config));
>> rte_config.mem_config = rte_mem_cfg_addr;
>> @@ -211,10 +221,11 @@ enum rte_iova_mode
>> * processes could later map the config into this exact location
>> */
>> rte_config.mem_config->mem_cfg_addr = (uintptr_t)
>> rte_mem_cfg_addr;
>> + return 0;
>> }
>>
>>
>
> <...>
>
> /* map it as read-only first */
>> mem_config = (struct rte_mem_config *) mmap(NULL,
>> sizeof(*mem_config),
>> PROT_READ, MAP_SHARED, mem_cfg_fd, 0);
>> - if (mem_config == MAP_FAILED)
>> - rte_panic("Cannot mmap memory for rte_config! error %i
>> (%s)\n",
>> - errno, strerror(errno));
>> + if (mem_config == MAP_FAILED) {
>> + mem_cfg_fd = -1;
>>
>
> Forgot close() here, i think.
>
> + RTE_LOG(CRIT, EAL, "%s(): Cannot mmap memory for
>> rte_config! error %i (%s)\n",
>> + __func__, errno, strerror(errno));
>> + return -1;
>> + }
>> rte_config.mem_config = mem_config;
>> +
>> + return 0;
>> }
>> /* reattach the shared config at exact memory location primary
>> process has it */
>>
>
> <...>
>
> + if (rte_config_init() != 0)
>> + return -1;
>> +
>> if (rte_eal_log_init(logid, internal_config.syslog_facility) <
>> 0) {
>> rte_eal_init_alert("Cannot init logging.");
>> rte_errno = ENOMEM;
>> @@ -914,6 +946,7 @@ static void rte_eal_init_alert(const char *msg)
>> */
>> if (pipe(lcore_config[i].pipe_master2slave) < 0)
>> rte_panic("Cannot create pipe\n");
>> +
>> if (pipe(lcore_config[i].pipe_slave2master) < 0)
>>
>
> Again, looks like unintended whitespace change.
>
> rte_panic("Cannot create pipe\n");
>>
>>
>
> Thanks Anatoly
Re: [dpdk-dev] [PATCH v7 08/11] eal: replace rte_panic instances in interrupts thread
> Just a general comment - i'm not too familiar with this code, but it looks > like all of these failures will only happen on thread init. Can we make > sure it starts? > > You can use similar approach to Olivier's (recently merged) thread > affinity patches, with pthread barriers etc. to ensure the thread has > initialized properly, pthread_cancel() it if it didn't, and return -1 on > thread init. Thanks for catching this one. I am now reverting it as well from this set, to be properly handled in a set dedicated to the threading issue
Re: [dpdk-dev] [PATCH v7 02/11] bond: replace rte_panic instances in bonding driver
> > > You mixed dpaa2 with bonding in this set. > Indeed. Thanks
