On 1 Jun 2022, at 9:02, Roi Dayan wrote:
> On 2022-05-31 5:34 PM, Eelco Chaudron 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 <[email protected]>
>> ---
>> 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
>> [email protected]
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
> thanks! also verified with simple ipv6 traffic.
>
> Acked-by: Roi Dayan <[email protected]>
>
> note this conflict with your other patch
> "[PATCH v4] odp_util: Fix parse_key_and_mask_to_match() vlan parsing"
Thanks for the review!! Yes, I noticed but did not want to make this dependent
as I think this needs to go in asap as Linux 5.18 is out.
//Eelco
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
