Skip to content

Commit

Permalink
checkpatch: Add rule to check for hardcoded table numbers.
Browse files Browse the repository at this point in the history
To avoid issues with hardcoded table numbers in future add rule
into check patch. The rule is only warning because there are still
legitimate use cases and not everything can be abstracted.

Signed-off-by: Ales Musil <[email protected]>
Acked-by: Mark Michelson <[email protected]>
Signed-off-by: Mark Michelson <[email protected]>
  • Loading branch information
almusil authored and putnopvut committed Mar 18, 2024
1 parent 348a296 commit 8840c6c
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 0 deletions.
39 changes: 39 additions & 0 deletions tests/checkpatch.at
Original file line number Diff line number Diff line change
Expand Up @@ -605,3 +605,42 @@ try_checkpatch \
Subject: netdev: This is a way to long commit summary and therefor it should report a WARNING!"

AT_CLEANUP

AT_SETUP([checkpatch - hardcoded table numbers])
try_checkpatch \
"COMMON_PATCH_HEADER([tests/something.at])
+table=12(ls_in_hairpin ), priority=1000 , match=(reg0[[14]] == 1), action=(next(pipeline=ingress, table=??);)
" \
"WARNING: Use of hardcoded table=<NUMBER> or resubmit=(,<NUMBER>) is discouraged in tests. Consider using MACRO instead.
#8 FILE: tests/something.at:1:
table=12(ls_in_hairpin ), priority=1000 , match=(reg0[[14]] == 1), action=(next(pipeline=ingress, table=??);)
"

try_checkpatch \
"COMMON_PATCH_HEADER([tests/something.at])
+table=??(ls_in_hairpin ), priority=1000 , match=(reg0[[14]] == 1), action=(next(pipeline=ingress, table=13);)
" \
"WARNING: Use of hardcoded table=<NUMBER> or resubmit=(,<NUMBER>) is discouraged in tests. Consider using MACRO instead.
#8 FILE: tests/something.at:1:
table=??(ls_in_hairpin ), priority=1000 , match=(reg0[[14]] == 1), action=(next(pipeline=ingress, table=13);)
"

try_checkpatch \
"COMMON_PATCH_HEADER([tests/something.at])
+priority=1100,ip,reg15=0x1,metadata=0x1,nw_src=10.0.0.1 actions=load:0x1->OXM_OF_PKT_REG4[[49]],resubmit(,47)
" \
"WARNING: Use of hardcoded table=<NUMBER> or resubmit=(,<NUMBER>) is discouraged in tests. Consider using MACRO instead.
#8 FILE: tests/something.at:1:
priority=1100,ip,reg15=0x1,metadata=0x1,nw_src=10.0.0.1 actions=load:0x1->OXM_OF_PKT_REG4[[49]],resubmit(,47)
"

try_checkpatch \
"COMMON_PATCH_HEADER([tests/something.at])
+C_H_E_C_K([as gw1 ovs-ofctl dump-flows br-int table=42 | grep "dl_dst=00:00:02:01:02:04" | wc -l], [0], [[1]])
" \
"WARNING: Use of hardcoded table=<NUMBER> or resubmit=(,<NUMBER>) is discouraged in tests. Consider using MACRO instead.
#8 FILE: tests/something.at:1:
C_H_E_C_K([as gw1 ovs-ofctl dump-flows br-int table=42 | grep "dl_dst=00:00:02:01:02:04" | wc -l], [0], [[1]])
"

AT_CLEANUP
12 changes: 12 additions & 0 deletions utilities/checkpatch.py
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ def reset_counters():
__parenthesized_constructs)
__regex_nonascii_characters = re.compile("[^\u0000-\u007f]")
__regex_efgrep = re.compile(r'.*[ef]grep.*$')
__regex_hardcoded_table = re.compile(r'.*(table=[0-9]+)|.*(resubmit\(,[0-9]+\))')

skip_leading_whitespace_check = False
skip_trailing_whitespace_check = False
Expand Down Expand Up @@ -371,6 +372,10 @@ def has_efgrep(line):
"""Returns TRUE if the current line contains 'egrep' or 'fgrep'."""
return __regex_efgrep.match(line) is not None

def has_hardcoded_table(line):
"""Return TRUE if the current line contains table=<NUMBER> or
resubmit(,<NUMBER>)"""
return __regex_hardcoded_table.match(line) is not None

def filter_comments(current_line, keep=False):
"""remove all of the c-style comments in a line"""
Expand Down Expand Up @@ -656,6 +661,13 @@ def empty_return(line):
'check': lambda x: has_efgrep(x),
'print':
lambda: print_error("grep -E/-F should be used instead of egrep/fgrep")},

{'regex': r'\.at$', 'match_name': None,
'check': lambda x: has_hardcoded_table(x),
'print':
lambda: print_warning("Use of hardcoded table=<NUMBER> or"
" resubmit=(,<NUMBER>) is discouraged in tests."
" Consider using MACRO instead.")},
]


Expand Down

0 comments on commit 8840c6c

Please sign in to comment.