David Ahern wrote: > On 2/22/18 6:02 AM, Serhey Popovych wrote: >> Now in iplink_parse() we use ->ifi_change and ->ifi_flags fields and >> plan to use ->ifi_index with upcoming change. >> >> Saving, restoring and reinitializing individual fields is error prone: >> using new field in iplink_parse() without updating callers in veth and >> vxcan will overwrite main device ifinfomsg data. >> >> Since @struct ifinfomsg is small enough with known sizeof() compiler may >> inline memcpy()/memset() with few load/store instructions. >> >> Signed-off-by: Serhey Popovych <serhe.popov...@gmail.com> >> --- >> ip/iplink_vxcan.c | 22 ++++++++-------------- >> ip/link_veth.c | 22 ++++++++-------------- >> 2 files changed, 16 insertions(+), 28 deletions(-) > > I don't agree that this change has any benefit. Only the flags and > change field are wanted; there is no need to save the entire struct, >
At the v0 (before submitted) I think so and just add ifi_index save/restore in addition to ifi_change and ifi_flags. But when look that memcpy() at 64-bit inlined to just two load/store instructions I decided to send memcpy()/memset() variant. If saving/restoring just ifi_index in addition to ifi_change and ifi_flags instead of full @struct ifinfomsg is preferred I will get rid of this change and add ifi_index to corresponding patch within series.
signature.asc
Description: OpenPGP digital signature