tree 94e9be8aca195fa2cb8f1e89ce064c9dbac5ef47
parent b4dbcce0524f6aca0a35910df0d6631e68343189
author Denis Ovsienko <denis@ovsienko.info> 1600876964 +0100
committer Denis Ovsienko <denis@ovsienko.info> 1600970325 +0100

OpenFlow: Modernize packet parsing style.

Most OpenFlow functions operated on the (cp, ep, len) tuple, returned a
pointer to tell the calling function where to decode next (which was not
a good idea because the calling functions had the values of cp and len in
the first place) and set the pointer to ep to indicate truncated data.
Change them to use the (cp, len) tuple, to be void and to use nd_trunc().
Update the header comment to explain this new convention.

Now if a low-level function gets an offset wrong, as in the bug recently
fixed in of10_queue_props_print(), and manages to return, the error will
not propagate into the calling function, which will be able to continue
the decoding.

Since cp does not have to be current anymore when the [void] function
returns, remove a number of cp increments just before the return.
Introduce OF_FWD(), which modifies both cp and len in sync, and
OF_CHK_FWD(), which also does a bounds check, and use these macros
instead of incrementing cp directly in every function that has both cp
and len arguments. Update the code to account for len that is
decrementing as cp is incrementing.

Commit ad69daa2 was fine for its time, but the introduction of the GET
macros had made it obsolete. Instead of reintroducing ep back merge
of_header_body_print() into openflow_print() to simplify the handling of
pointers, sizes and error conditions, all in the style above.

While at it, subtract OF_HEADER_FIXLEN from the declared OpenFlow message
length before passing it to of10_header_body_print() and make the latter
the only function that has to know about the common header size (clarify
it in the comment). This and the sufficiently current cp and len
disencumber the following functions of type-specific length calculations:
of10_features_reply_print(), of10_flow_mod_print(),
of10_stats_request_print(), of10_stats_reply_print(),
of10_packet_out_print(), of10_packet_in_print() and of10_error_print().

The resulting code in most cases addresses the not yet processed part of
the declared structure with just cp and len: nextfunc(ndo, cp, len) and
ND_TCHECK_LEN(cp, len) do the right thing regardless of how far the
current decoder has made it into the current structure. This makes all
the cp0 and len0 snapshot variables obsolete, so remove them too.

Update a test case output: the missing closing quote now correctly means
that the snapshot end is inside the "data" field of a BSN_SHELL_COMMAND
vendor message, not after it.