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.
>
> Reported-by: Alexander Popov <alex.po...@linux.com>
> Signed-off-by: Tom Herbert <t...@quantonium.net>
> ---
> 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
...
