On Tue, 5 Sep 2017 13:51:45 +0800, Yang, Yi wrote: > But if we follow your way, how does nla_for_each_nested handle such > pattern? > > attribute1 > attribute1_mask > attribute2 > attribute2_mask > attribute3 > attribute3_mask
Uh? That will just work. Note that nla len contains the size of the whole attribute, i.e. includes both fields. > I don't think this can increase performance and readability. Do you realize you're stating that not copying data around in hot path can't increase performance? ;-) > 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. Except that what you did is so unexpected to netlink that you had to go out of your way to parse it. Those two memcpys speak for themselves. > If we consider OVS_KEY_ATTR_NSH as a big attribute, current way is > precisely following current design pattern Do you have an idea how nested netlink attributes work? It's very well defined. What you're doing is breaking the definition. You can't do that. The (non-nested) attributes contain header followed by data. The data is a single value or it is a struct. In ovs for masked actions, attributes contain a struct of two fields (value and mask). The struct access is open coded but it's still a struct. Now, nested attributes are very well defined: it's a nla header followed by a stream of attributes. There's no requirement on the order of the attributes. Do you see how you're breaking this assumption? You expect that each attribute is present exactly twice, once among first half of attributes, once among second half of attributes. You don't check that this weird assumption is adhered to. You don't even check that the point where you split the data is between attributes! You may very well be splitting an attribute in half and interpreting its data as nla headers. Consider your proposal NACKed from my side. Jiri
