Re: [PATCH net-next 1/2] flow_dissector: Cleanup control flow
From: Simon Horman <hidden>
Date: 2017-09-01 12:26:55
Hi Tom, On Thu, Aug 31, 2017 at 03:22:38PM -0700, Tom Herbert wrote:
__skb_flow_dissect is riddled with gotos that make discerning the flow, debugging, and extending the capability difficult. This patch reorganizes things so that we only perform goto's after the two main switch statements (no gotos within the cases now). It also eliminates several goto labels so that there are only two labels that can be target for goto.
I agree that the flow of __skb_flow_dissect() is difficult to follow but its not obvious that this significant change in terms of loc takes us to a better place. Maybe it makes follow-up work easier. If so perhaps it should be motivated along those lines. In any case I won't stand in the way of this change but I did want to throw my 2c worth in.
quoted hunk ↗ jump to hunk
Reported-by: Alexander Popov <redacted> Signed-off-by: Tom Herbert <redacted> --- include/net/flow_dissector.h | 9 ++ net/core/flow_dissector.c | 225 ++++++++++++++++++++++++++++--------------- 2 files changed, 156 insertions(+), 78 deletions(-)diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h index e2663e900b0a..c358c3ff6acc 100644 --- a/include/net/flow_dissector.h +++ b/include/net/flow_dissector.h@@ -19,6 +19,15 @@ struct flow_dissector_key_control { #define FLOW_DIS_FIRST_FRAG BIT(1) #define FLOW_DIS_ENCAPSULATION BIT(2) +enum flow_dissect_ret { + FLOW_DISSECT_RET_OUT_GOOD, + FLOW_DISSECT_RET_OUT_BAD, + FLOW_DISSECT_RET_PROTO_AGAIN, + FLOW_DISSECT_RET_IPPROTO_AGAIN, + FLOW_DISSECT_RET_IPPROTO_AGAIN_EH, + FLOW_DISSECT_RET_CONTINUE, +};
Minor nit: My reading is that this patch does not seem to differentiate between the handling of FLOW_DISSECT_RET_IPPROTO_AGAIN and FLOW_DISSECT_RET_IPPROTO_AGAIN_EH. Perhaps it would be better to add FLOW_DISSECT_RET_IPPROTO_AGAIN_EH in the following patch where it is used.
+ /** * struct flow_dissector_key_basic: * @thoff: Transport header offset
...