On Tue, May 31, 2022 at 10:35 AM Eelco Chaudron <echau...@redhat.com> wrote:
>
> When processing netlink messages, we should ignore unknown OVS_KEY_ATTR
> as we can assume if newer attributes are present, they are backward
> compatible.
>
> This patch also updates the existing comments to better explain what
> happens in the error cases. At this place in the code, we can not
> ignore the TOO_LITTLE/MUCH errors as some instances could have
> canceled processing leaving the returning match data incomplete.
>
> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2089331
> Signed-off-by: Eelco Chaudron <echau...@redhat.com>
Reviewed-by: Michael Santana <msant...@redhat.com>
I would probably just have gotten rid of out_of_range_attr entirely in
the code base, but I guess this requires a little more work and has a
higher chance of breaking things. Your approach is safer and it gets
the job done
> ---
> lib/odp-util.c | 34 +++++++++++++++++++++++++---------
> 1 file changed, 25 insertions(+), 9 deletions(-)
>
> diff --git a/lib/odp-util.c b/lib/odp-util.c
> index 40e89a7cf..0e6614bd8 100644
> --- a/lib/odp-util.c
> +++ b/lib/odp-util.c
> @@ -7263,6 +7263,14 @@ parse_8021q_onward(const struct nlattr
> *attrs[OVS_KEY_ATTR_MAX + 1],
> }
> expected_attrs = 0;
>
> + /* For OVS to be backward compatible with newer datapath
> + * implementations, we should ignore out of range attributes. */
> + if (out_of_range_attr) {
> + VLOG_DBG("Flow key decode found unknown OVS_KEY_ATTR, %d",
> + out_of_range_attr);
> + out_of_range_attr = 0;
> + }
> +
> if (!parse_ethertype(attrs, present_attrs, &expected_attrs,
> flow, src_flow, errorp)) {
> return ODP_FIT_ERROR;
> @@ -7312,6 +7320,14 @@ odp_flow_key_to_flow__(const struct nlattr *key,
> size_t key_len,
> }
> expected_attrs = 0;
>
> + /* For OVS to be backward compatible with newer datapath implementations,
> + * we should ignore out of range attributes. */
> + if (out_of_range_attr) {
> + VLOG_DBG("Flow key decode found unknown OVS_KEY_ATTR, %d",
> + out_of_range_attr);
> + out_of_range_attr = 0;
> + }
> +
> /* Metadata. */
> if (present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_RECIRC_ID)) {
> flow->recirc_id = nl_attr_get_u32(attrs[OVS_KEY_ATTR_RECIRC_ID]);
> @@ -7546,10 +7562,12 @@ parse_key_and_mask_to_match(const struct nlattr *key,
> size_t key_len,
>
> fitness = odp_flow_key_to_flow(key, key_len, &match->flow, NULL);
> if (fitness) {
> - /* This should not happen: it indicates that
> - * odp_flow_key_from_flow() and odp_flow_key_to_flow() disagree on
> - * the acceptable form of a flow. Log the problem as an error,
> - * with enough details to enable debugging. */
> + /* This will happen when the odp_flow_key_to_flow() function can't
> + * parse the netlink message to a match structure. It will return
> + * ODP_FIT_TOO_LITTLE if there is not enough information to parse the
> + * content successfully, ODP_FIT_TOO_MUCH if there is too much
> netlink
> + * data and we do not know how to safely ignore it, and ODP_FIT_ERROR
> + * in any other case. */
> static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
>
> if (!VLOG_DROP_ERR(&rl)) {
> @@ -7557,7 +7575,8 @@ parse_key_and_mask_to_match(const struct nlattr *key,
> size_t key_len,
>
> ds_init(&s);
> odp_flow_format(key, key_len, NULL, 0, NULL, &s, true);
> - VLOG_ERR("internal error parsing flow key %s", ds_cstr(&s));
> + VLOG_ERR("internal error parsing flow key %s (%s)",
> + ds_cstr(&s), odp_key_fitness_to_string(fitness));
> ds_destroy(&s);
> }
>
> @@ -7567,10 +7586,7 @@ parse_key_and_mask_to_match(const struct nlattr *key,
> size_t key_len,
> fitness = odp_flow_key_to_mask(mask, mask_len, &match->wc, &match->flow,
> NULL);
> if (fitness) {
> - /* This should not happen: it indicates that
> - * odp_flow_key_from_mask() and odp_flow_key_to_mask()
> - * disagree on the acceptable form of a mask. Log the problem
> - * as an error, with enough details to enable debugging. */
> + /* This should not happen, see comment above. */
> static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
>
> if (!VLOG_DROP_ERR(&rl)) {
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
