| Message ID | 1495894476-9726-4-git-send-email-vyasevic@redhat.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | stephen hemminger |
| Headers | show |
On 5/27/17 8:14 AM, Vladislav Yasevich wrote: > diff --git a/ip/ipaddress.c b/ip/ipaddress.c > index b8d9c7d..c6e7413 100644 > --- a/ip/ipaddress.c > +++ b/ip/ipaddress.c > @@ -753,6 +753,24 @@ int print_linkinfo_brief(const struct sockaddr_nl *who, > return 0; > } > > +static const char *netdev_events[] = {"NONE", I would prefer ifla_events rather than netdev just to reinforce the separation that an IFLA_EVENT does not necessarily correspond to a NETDEV event.
On Sat, 27 May 2017 10:14:36 -0400 Vladislav Yasevich <vyasevich@gmail.com> wrote: > > +static const char *netdev_events[] = {"NONE", > + "REBOOT", > + "FEATURE CHANGE", > + "BONDING FAILOVER", > + "NOTIFY PEERS", > + "RESEND IGMP", > + "BONDING OPTION"}; Overall this looks fine, I will pickup the if_link.h from net-next. One stylistic change. Please add simple line break, and initialize by value: static const char *netdev_events[] = { [IFLA_EVENT_NONE] = "NONE", ... Do you want some prefix or bounding around the event output? Also a little concerned that the output format change may break some program could the new output be at the end of the line?
On 05/30/2017 01:12 PM, Stephen Hemminger wrote: > On Sat, 27 May 2017 10:14:36 -0400 > Vladislav Yasevich <vyasevich@gmail.com> wrote: > >> >> +static const char *netdev_events[] = {"NONE", >> + "REBOOT", >> + "FEATURE CHANGE", >> + "BONDING FAILOVER", >> + "NOTIFY PEERS", >> + "RESEND IGMP", >> + "BONDING OPTION"}; > > Overall this looks fine, I will pickup the if_link.h from net-next. > > One stylistic change. > > Please add simple line break, and initialize by value: > > static const char *netdev_events[] = { > [IFLA_EVENT_NONE] = "NONE", > ... > > Do you want some prefix or bounding around the event output? Don't really care about output from my side. If you think some prefix would be good, I can surely add it. > Also a little concerned that the output format change may break some program > could the new output be at the end of the line? > I can try moving it to the end. Thanks -vlad
On 05/30/2017 02:26 PM, Vlad Yasevich wrote: > On 05/30/2017 01:12 PM, Stephen Hemminger wrote: >> On Sat, 27 May 2017 10:14:36 -0400 >> Vladislav Yasevich <vyasevich@gmail.com> wrote: >> >>> >>> +static const char *netdev_events[] = {"NONE", >>> + "REBOOT", >>> + "FEATURE CHANGE", >>> + "BONDING FAILOVER", >>> + "NOTIFY PEERS", >>> + "RESEND IGMP", >>> + "BONDING OPTION"}; >> >> Overall this looks fine, I will pickup the if_link.h from net-next. >> >> One stylistic change. >> >> Please add simple line break, and initialize by value: >> >> static const char *netdev_events[] = { >> [IFLA_EVENT_NONE] = "NONE", >> ... >> >> Do you want some prefix or bounding around the event output? > > Don't really care about output from my side. If you think some prefix > would be good, I can surely add it. > >> Also a little concerned that the output format change may break some program >> could the new output be at the end of the line? >> > > I can try moving it to the end. > Hi Stephen So, I looked again at this patch and the output change I am proposing would look something like this 'ip monitor': 1: lo: <LOOPBACK,UP,LOWER_UP> mtu 6500 qdisc noqueue state UNKNOWN group default event FEATURE CHANGE link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00 It's already at the end of the like ant has an 'event' prefix similar to all the other entries on the top line. Does that look OK? I don't think that would break anything. Thanks -vlad
diff --git a/include/linux/if_link.h b/include/linux/if_link.h index 5a3a048..c0a6769 100644 --- a/include/linux/if_link.h +++ b/include/linux/if_link.h @@ -157,6 +157,7 @@ enum { IFLA_GSO_MAX_SIZE, IFLA_PAD, IFLA_XDP, + IFLA_EVENT, __IFLA_MAX }; @@ -909,4 +910,14 @@ enum { #define IFLA_XDP_MAX (__IFLA_XDP_MAX - 1) +enum { + IFLA_EVENT_NONE, + IFLA_EVENT_REBOOT, /* internal reset / reboot */ + IFLA_EVENT_FEATURES, /* change in offload features */ + IFLA_EVENT_BONDING_FAILOVER, /* hange in active slave */ + IFLA_EVENT_NOTIFY_PEERS, /* re-sent grat. arp/ndisc */ + IFLA_EVENT_IGMP_RESEND, /* re-sent IGMP JOIN */ + IFLA_EVENT_BONDING_OPTIONS, /* change in bonding options */ +}; + #endif /* _LINUX_IF_LINK_H */ diff --git a/ip/ipaddress.c b/ip/ipaddress.c index b8d9c7d..c6e7413 100644 --- a/ip/ipaddress.c +++ b/ip/ipaddress.c @@ -753,6 +753,24 @@ int print_linkinfo_brief(const struct sockaddr_nl *who, return 0; } +static const char *netdev_events[] = {"NONE", + "REBOOT", + "FEATURE CHANGE", + "BONDING FAILOVER", + "NOTIFY PEERS", + "RESEND IGMP", + "BONDING OPTION"}; + +static void print_dev_event(FILE *f, __u32 event) +{ + if (event >= ARRAY_SIZE(netdev_events)) + fprintf(f, "event %d ", event); + else { + if (event) + fprintf(f, "event %s ", netdev_events[event]); + } +} + int print_linkinfo(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg) { @@ -858,6 +876,9 @@ int print_linkinfo(const struct sockaddr_nl *who, if (filter.showqueue) print_queuelen(fp, tb); + if (tb[IFLA_EVENT]) + print_dev_event(fp, rta_getattr_u32(tb[IFLA_EVENT])); + if (!filter.family || filter.family == AF_PACKET || show_details) { SPRINT_BUF(b1); fprintf(fp, "%s", _SL_);
Add IFLA_EVENT handling so that event types can be viewed with 'monitor' command. This gives a little more information for why a given message was receivied. Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com> --- include/linux/if_link.h | 11 +++++++++++ ip/ipaddress.c | 21 +++++++++++++++++++++ 2 files changed, 32 insertions(+)