On Mon, Sep 04, 2017 at 08:57:44PM +0800, Jiri Benc wrote: > On Mon, 4 Sep 2017 20:09:07 +0800, Yang, Yi wrote: > > So we must do many changes if we want to break this assumption. > > We may do as many changes as we want to. This is uAPI we're talking > about and we need to get it right since the beginning. Sure, it may > mean that some user space programs need some changes in order to make > use of the new features. That happens every day. > > I also don't understand where's the problem. It's very easy to check > for NLA_F_NESTED and generically act based on that in the function you > quoted. Just call a different function than format_odp_key_attr to > handle ovs_nsh_key_attr attributes in case the nested flag is set and > the attribute is OVS_KEY_ATTR_NSH and you're done. You'll need such > function anyway, it's not much different code size wise to call it from > format_odp_key_attr or from format_odp_action.
But if we follow your way, how does nla_for_each_nested handle such pattern? attribute1 attribute1_mask attribute2 attribute2_mask attribute3 attribute3_mask I don't think this can increase performance and readability. In current way, we just call nla_for_each_nested twice one is for attribute1 attribute2 attribute3 another is for attribute1_mask attribute2_mask attribute3_mask if we use one function to handle both attributes and masks, I can't see any substantial difference between two ways as far as the performance is concerned. So my proposal is we needn't introduce special handling case for OVS_KEY_ATTR_NSH in OVS_ACTION_ATTR_SET_MASKED, that will open Pandora's box. If we consider OVS_KEY_ATTR_NSH as a big attribute, current way is precisely following current design pattern attribute mask > > Jiri
