diff mbox series

[net-next,v3,3/3] net: sched: Add initial TC error skb drop reasons

Message ID 20231205205030.3119672-4-victor@mojatatu.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series net: sched: Make tc-related drop reason more flexible for remaining qdiscs | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 6113 this patch: 6113
netdev/cc_maintainers success CCed 7 of 7 maintainers
netdev/build_clang success Errors and warnings before: 2361 this patch: 2361
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 6459 this patch: 6459
netdev/checkpatch warning WARNING: line length of 100 exceeds 80 columns WARNING: line length of 87 exceeds 80 columns WARNING: line length of 88 exceeds 80 columns WARNING: line length of 92 exceeds 80 columns
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Victor Nogueira Dec. 5, 2023, 8:50 p.m. UTC 
Continue expanding Daniel's patch by adding new skb drop reasons that
are idiosyncratic to TC.

More specifically:

- SKB_DROP_REASON_TC_EXT_COOKIE_NOTFOUND: tc cookie was looked up using
  ext, but was not found.

- SKB_DROP_REASON_TC_COOKIE_EXT_MISMATCH: tc ext was looked up using cookie
  and either was not found or different from expected.

- SKB_DROP_REASON_TC_CHAIN_NOTFOUND: tc chain lookup failed.

- SKB_DROP_REASON_TC_RECLASSIFY_LOOP: tc exceeded max reclassify loop
  iterations

Signed-off-by: Victor Nogueira <victor@mojatatu.com>
---
 include/net/dropreason-core.h | 30 +++++++++++++++++++++++++++---
 net/sched/act_api.c           |  3 ++-
 net/sched/cls_api.c           | 22 ++++++++++++++--------
 3 files changed, 43 insertions(+), 12 deletions(-)

Comments

Simon Horman Dec. 11, 2023, 1:38 p.m. UTC | #1 
On Tue, Dec 05, 2023 at 05:50:30PM -0300, Victor Nogueira wrote:
> Continue expanding Daniel's patch by adding new skb drop reasons that
> are idiosyncratic to TC.
> 
> More specifically:
> 
> - SKB_DROP_REASON_TC_EXT_COOKIE_NOTFOUND: tc cookie was looked up using
>   ext, but was not found.
> 
> - SKB_DROP_REASON_TC_COOKIE_EXT_MISMATCH: tc ext was looked up using cookie
>   and either was not found or different from expected.
> 
> - SKB_DROP_REASON_TC_CHAIN_NOTFOUND: tc chain lookup failed.
> 
> - SKB_DROP_REASON_TC_RECLASSIFY_LOOP: tc exceeded max reclassify loop
>   iterations
> 
> Signed-off-by: Victor Nogueira <victor@mojatatu.com>

Reviewed-by: Simon Horman <horms@kernel.org>
Jakub Kicinski Dec. 12, 2023, 2:30 a.m. UTC | #2 
On Tue,  5 Dec 2023 17:50:30 -0300 Victor Nogueira wrote:
> +	/**
> +	 * @SKB_DROP_REASON_TC_EXT_COOKIE_NOTFOUND: tc cookie was looked up
> +	 * using ext, but was not found.
> +	 */
> +	SKB_DROP_REASON_TC_EXT_COOKIE_NOTFOUND,
> +	/**
> +	 * @SKB_DROP_REASON_TC_COOKIE_EXT_MISMATCH: tc ext was lookup using
> +	 * cookie and either was not found or different from expected.
> +	 */
> +	SKB_DROP_REASON_TC_COOKIE_EXT_MISMATCH,
> +	/**
> +	 * @SKB_DROP_REASON_TC_COOKIE_MISMATCH: tc cookie available but was
> +	 * unable to match to filter.
> +	 */
> +	SKB_DROP_REASON_TC_COOKIE_MISMATCH,

Do we really need 3 reasons for COOKIE?

Also cookie here is offload state thing right? I wonder how many admins
/ SREs would be able to figure out what's going on based on this kdoc :S
Let alone if it's a configuration problem or a race condition...
Victor Nogueira Dec. 13, 2023, 6:25 p.m. UTC | #3 
On 11/12/2023 23:30, Jakub Kicinski wrote:
> On Tue,  5 Dec 2023 17:50:30 -0300 Victor Nogueira wrote:
>> +	/**
>> +	 * @SKB_DROP_REASON_TC_EXT_COOKIE_NOTFOUND: tc cookie was looked up
>> +	 * using ext, but was not found.
>> +	 */
>> +	SKB_DROP_REASON_TC_EXT_COOKIE_NOTFOUND,
>> +	/**
>> +	 * @SKB_DROP_REASON_TC_COOKIE_EXT_MISMATCH: tc ext was lookup using
>> +	 * cookie and either was not found or different from expected.
>> +	 */
>> +	SKB_DROP_REASON_TC_COOKIE_EXT_MISMATCH,
>> +	/**
>> +	 * @SKB_DROP_REASON_TC_COOKIE_MISMATCH: tc cookie available but was
>> +	 * unable to match to filter.
>> +	 */
>> +	SKB_DROP_REASON_TC_COOKIE_MISMATCH,
> 
> Do we really need 3 reasons for COOKIE?
> 
> Also cookie here is offload state thing right? I wonder how many admins
> / SREs would be able to figure out what's going on based on this kdoc :S
> Let alone if it's a configuration problem or a race condition...

Yeah, probably overkill. Will merge into one.

cheers,
Victor
diff mbox series

Patch

diff --git a/include/net/dropreason-core.h b/include/net/dropreason-core.h
index 3c70ad53a49c..fa6ace8f1611 100644
--- a/include/net/dropreason-core.h
+++ b/include/net/dropreason-core.h
@@ -85,7 +85,11 @@ 
 	FN(IPV6_NDISC_BAD_OPTIONS)	\
 	FN(IPV6_NDISC_NS_OTHERHOST)	\
 	FN(QUEUE_PURGE)			\
-	FN(TC_ERROR)			\
+	FN(TC_EXT_COOKIE_NOTFOUND)	\
+	FN(TC_COOKIE_EXT_MISMATCH)	\
+	FN(TC_COOKIE_MISMATCH)		\
+	FN(TC_CHAIN_NOTFOUND)		\
+	FN(TC_RECLASSIFY_LOOP)		\
 	FNe(MAX)
 
 /**
@@ -376,8 +380,28 @@  enum skb_drop_reason {
 	SKB_DROP_REASON_IPV6_NDISC_NS_OTHERHOST,
 	/** @SKB_DROP_REASON_QUEUE_PURGE: bulk free. */
 	SKB_DROP_REASON_QUEUE_PURGE,
-	/** @SKB_DROP_REASON_TC_ERROR: generic internal tc error. */
-	SKB_DROP_REASON_TC_ERROR,
+	/**
+	 * @SKB_DROP_REASON_TC_EXT_COOKIE_NOTFOUND: tc cookie was looked up
+	 * using ext, but was not found.
+	 */
+	SKB_DROP_REASON_TC_EXT_COOKIE_NOTFOUND,
+	/**
+	 * @SKB_DROP_REASON_TC_COOKIE_EXT_MISMATCH: tc ext was lookup using
+	 * cookie and either was not found or different from expected.
+	 */
+	SKB_DROP_REASON_TC_COOKIE_EXT_MISMATCH,
+	/**
+	 * @SKB_DROP_REASON_TC_COOKIE_MISMATCH: tc cookie available but was
+	 * unable to match to filter.
+	 */
+	SKB_DROP_REASON_TC_COOKIE_MISMATCH,
+	/** @SKB_DROP_REASON_TC_CHAIN_NOTFOUND: tc chain lookup failed. */
+	SKB_DROP_REASON_TC_CHAIN_NOTFOUND,
+	/**
+	 * @SKB_DROP_REASON_TC_RECLASSIFY_LOOP: tc exceeded max reclassify loop
+	 * iterations.
+	 */
+	SKB_DROP_REASON_TC_RECLASSIFY_LOOP,
 	/**
 	 * @SKB_DROP_REASON_MAX: the maximum of core drop reasons, which
 	 * shouldn't be used as a real 'reason' - only for tracing code gen
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index f2b136ce9282..ba5aad9be161 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -1098,7 +1098,8 @@  int tcf_action_exec(struct sk_buff *skb, struct tc_action **actions,
 			}
 		} else if (TC_ACT_EXT_CMP(ret, TC_ACT_GOTO_CHAIN)) {
 			if (unlikely(!rcu_access_pointer(a->goto_chain))) {
-				tcf_set_drop_reason(skb, SKB_DROP_REASON_TC_ERROR);
+				tcf_set_drop_reason(skb,
+						    SKB_DROP_REASON_TC_CHAIN_NOTFOUND);
 				return TC_ACT_SHOT;
 			}
 			tcf_action_goto_chain_exec(a, res);
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 32457a236d77..5012fc0a24a1 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -1682,13 +1682,15 @@  static inline int __tcf_classify(struct sk_buff *skb,
 			 */
 			if (unlikely(n->tp != tp || n->tp->chain != n->chain ||
 				     !tp->ops->get_exts)) {
-				tcf_set_drop_reason(skb, SKB_DROP_REASON_TC_ERROR);
+				tcf_set_drop_reason(skb,
+						    SKB_DROP_REASON_TC_COOKIE_MISMATCH);
 				return TC_ACT_SHOT;
 			}
 
 			exts = tp->ops->get_exts(tp, n->handle);
 			if (unlikely(!exts || n->exts != exts)) {
-				tcf_set_drop_reason(skb, SKB_DROP_REASON_TC_ERROR);
+				tcf_set_drop_reason(skb,
+						    SKB_DROP_REASON_TC_COOKIE_EXT_MISMATCH);
 				return TC_ACT_SHOT;
 			}
 
@@ -1717,7 +1719,8 @@  static inline int __tcf_classify(struct sk_buff *skb,
 	}
 
 	if (unlikely(n)) {
-		tcf_set_drop_reason(skb, SKB_DROP_REASON_TC_ERROR);
+		tcf_set_drop_reason(skb,
+				    SKB_DROP_REASON_TC_COOKIE_MISMATCH);
 		return TC_ACT_SHOT;
 	}
 
@@ -1729,7 +1732,8 @@  static inline int __tcf_classify(struct sk_buff *skb,
 				       tp->chain->block->index,
 				       tp->prio & 0xffff,
 				       ntohs(tp->protocol));
-		tcf_set_drop_reason(skb, SKB_DROP_REASON_TC_ERROR);
+		tcf_set_drop_reason(skb,
+				    SKB_DROP_REASON_TC_RECLASSIFY_LOOP);
 		return TC_ACT_SHOT;
 	}
 
@@ -1767,7 +1771,8 @@  int tcf_classify(struct sk_buff *skb,
 				n = tcf_exts_miss_cookie_lookup(ext->act_miss_cookie,
 								&act_index);
 				if (!n) {
-					tcf_set_drop_reason(skb, SKB_DROP_REASON_TC_ERROR);
+					tcf_set_drop_reason(skb,
+							    SKB_DROP_REASON_TC_EXT_COOKIE_NOTFOUND);
 					return TC_ACT_SHOT;
 				}
 
@@ -1778,7 +1783,9 @@  int tcf_classify(struct sk_buff *skb,
 
 			fchain = tcf_chain_lookup_rcu(block, chain);
 			if (!fchain) {
-				tcf_set_drop_reason(skb, SKB_DROP_REASON_TC_ERROR);
+				tcf_set_drop_reason(skb,
+						    SKB_DROP_REASON_TC_CHAIN_NOTFOUND);
+
 				return TC_ACT_SHOT;
 			}
 
@@ -1800,10 +1807,9 @@  int tcf_classify(struct sk_buff *skb,
 
 			ext = tc_skb_ext_alloc(skb);
 			if (WARN_ON_ONCE(!ext)) {
-				tcf_set_drop_reason(skb, SKB_DROP_REASON_TC_ERROR);
+				tcf_set_drop_reason(skb, SKB_DROP_REASON_NOMEM);
 				return TC_ACT_SHOT;
 			}
-
 			ext->chain = last_executed_chain;
 			ext->mru = cb->mru;
 			ext->post_ct = cb->post_ct;