Re: [PATCH iproute2-next v3 5/8] veth,vxcan: Save/reinitialize/restore whole @struct ifinfomsg

David Ahern wrote:

On 2/22/18 6:02 AM, Serhey Popovych wrote:

quoted

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 <redacted>
---
 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,

Struct is small enough, only ifi_change and ifi_flags being used right
now, but next patch also changes ifi_index. Remaining values are fit in
single u32 (ifi_family, ifi_type and pad). So we copy only one extra u32
at the moment. Later if something gets changed and iplink_parse() starts
using ifi_family, ifi_type, ifi_pad... we do not need to modify vxcan
and veth modules.