From: Stephen Hemminger <stephen@networkplumber.org>
To: Fernando Fernandez Mancera <fmancera@suse.de>
Cc: netdev@vger.kernel.org, dsahern@kernel.org, davem@davemloft.net,
edumazet@google.com, kuba@kernel.org, pabeni@redhat.com,
horms@kernel.org
Subject: Re: [PATCH iproute2-next] ipaddress: add support for showing IPv4 devconf attributes
Date: Sun, 14 Jun 2026 08:55:56 -0700 [thread overview]
Message-ID: <20260614085556.1efda1eb@phoenix.local> (raw)
In-Reply-To: <3e4a425e-0f58-48dc-a2bc-88fd6eb4a302@suse.de>
On Sat, 13 Jun 2026 09:22:38 +0200
Fernando Fernandez Mancera <fmancera@suse.de> wrote:
> On 6/13/26 8:41 AM, Fernando Fernandez Mancera wrote:
> > On 6/13/26 4:29 AM, Stephen Hemminger wrote:
> >> On Sat, 13 Jun 2026 01:17:22 +0200
> >> Fernando Fernandez Mancera <fmancera@suse.de> wrote:
> >>
> >>> tatic void print_inet(FILE *fp, struct rtattr *inet_attr)
> >>> +{
> >>> + struct rtattr *tb[IFLA_INET_MAX + 1];
> >>> +
> >>> + parse_rtattr_nested(tb, IFLA_INET_MAX, inet_attr);
> >>> +
> >>> + if (tb[IFLA_INET_CONF] && show_details) {
> >>> + int *conf = RTA_DATA(tb[IFLA_INET_CONF]);
> >>> + int max_elements = RTA_PAYLOAD(tb[IFLA_INET_CONF]) /
> >>> sizeof(int);
> >>> +
> >>> + if (max_elements >= IPV4_DEVCONF_FORWARDING)
> >>> + print_string(PRINT_ANY, "forwarding", "forwarding %s ",
> >>> + conf[IPV4_DEVCONF_FORWARDING - 1] ? "on" : "off");
> >>> +
> >>> + if (max_elements >= IPV4_DEVCONF_MC_FORWARDING)
> >>> + print_string(PRINT_ANY, "mc_forwarding", "mc_forwarding
> >>> %s ",
> >>> + conf[IPV4_DEVCONF_MC_FORWARDING - 1] ? "on" :
> >>> "off");
> >>> +
> >>> + if (max_elements >= IPV4_DEVCONF_PROXY_ARP)
> >>> + print_string(PRINT_ANY, "proxy_arp", "proxy_arp %s ",
> >>> + conf[IPV4_DEVCONF_PROXY_ARP - 1] ? "on" : "off");
> >>> +
> >>> + if (max_elements >= IPV4_DEVCONF_ACCEPT_REDIRECTS)
> >>> + print_string(PRINT_ANY, "accept_redirects",
> >>> + "accept_redirects %s ",
> >>> + conf[IPV4_DEVCONF_ACCEPT_REDIRECTS - 1] ?
> >>> "on" : "off");
> >>> +
> >>> + if (max_elements >= IPV4_DEVCONF_SECURE_REDIRECTS)
> >>> + print_string(PRINT_ANY, "secure_redirects",
> >>> + "secure_redirects %s ",
> >>> + conf[IPV4_DEVCONF_SECURE_REDIRECTS - 1] ?
> >>> "on" : "off");
> >>> +
> >>> + if (max_elements >= IPV4_DEVCONF_SEND_REDIRECTS)
> >>> + print_string(PRINT_ANY, "send_redirects",
> >>> "send_redirects %s ",
> >>> + conf[IPV4_DEVCONF_SEND_REDIRECTS - 1] ? "on" :
> >>> "off");
> >>> +
> >>> + if (max_elements >= IPV4_DEVCONF_SHARED_MEDIA)
> >>> + print_string(PRINT_ANY, "shared_media", "shared_media %s ",
> >>> + conf[IPV4_DEVCONF_SHARED_MEDIA - 1] ? "on" :
> >>> "off");
> >>> +
> >>> + if (max_elements >= IPV4_DEVCONF_RP_FILTER)
> >>> + print_int(PRINT_ANY, "rp_filter", "rp_filter %d ",
> >>> + conf[IPV4_DEVCONF_RP_FILTER - 1]);
> >>> +
> >>> + if (max_elements >= IPV4_DEVCONF_ACCEPT_SOURCE_ROUTE)
> >>> + print_string(PRINT_ANY, "accept_source_route",
> >>> + "accept_source_route %s ",
> >>> + conf[IPV4_DEVCONF_ACCEPT_SOURCE_ROUTE - 1] ?
> >>> "on" : "off");
> >>> +
> >>> + if (max_elements >= IPV4_DEVCONF_BOOTP_RELAY)
> >>> + print_string(PRINT_ANY, "bootp_relay", "bootp_relay %s ",
> >>> + conf[IPV4_DEVCONF_BOOTP_RELAY - 1] ? "on" :
> >>> "off");
> >>> +
> >>> + if (max_elements >= IPV4_DEVCONF_LOG_MARTIANS)
> >>> + print_string(PRINT_ANY, "log_martians", "log_martians %s ",
> >>> + conf[IPV4_DEVCONF_LOG_MARTIANS - 1] ? "on" :
> >>> "off");
> >>> +
> >>> + if (max_elements >= IPV4_DEVCONF_TAG)
> >>> + print_int(PRINT_ANY, "tag", "tag %d ",
> >>> + conf[IPV4_DEVCONF_TAG - 1]);
> >>> +
> >>> + if (max_elements >= IPV4_DEVCONF_ARPFILTER)
> >>> + print_string(PRINT_ANY, "arpfilter", "arpfilter %s ",
> >>> + conf[IPV4_DEVCONF_ARPFILTER - 1] ? "on" : "off");
> >>> +
> >>> + if (max_elements >= IPV4_DEVCONF_MEDIUM_ID)
> >>> + print_int(PRINT_ANY, "medium_id", "medium_id %d ",
> >>> + conf[IPV4_DEVCONF_MEDIUM_ID - 1]);
> >>> +
> >>> + if (max_elements >= IPV4_DEVCONF_NOXFRM)
> >>> + print_string(PRINT_ANY, "noxfrm", "noxfrm %s ",
> >>> + conf[IPV4_DEVCONF_NOXFRM - 1] ? "on" : "off");
> >>> +
> >>> + if (max_elements >= IPV4_DEVCONF_NOPOLICY)
> >>> + print_string(PRINT_ANY, "nopolicy", "nopolicy %s ",
> >>> + conf[IPV4_DEVCONF_NOPOLICY - 1] ? "on" : "off");
> >>> +
> >>> + if (max_elements >= IPV4_DEVCONF_FORCE_IGMP_VERSION)
> >>> + print_int(PRINT_ANY, "force_igmp_version",
> >>> "force_igmp_version %d ",
> >>> + conf[IPV4_DEVCONF_FORCE_IGMP_VERSION - 1]);
> >>> +
> >>> + if (max_elements >= IPV4_DEVCONF_ARP_ANNOUNCE)
> >>> + print_int(PRINT_ANY, "arp_announce", "arp_announce %d ",
> >>> + conf[IPV4_DEVCONF_ARP_ANNOUNCE - 1]);
> >>> +
> >>> + if (max_elements >= IPV4_DEVCONF_ARP_IGNORE)
> >>> + print_int(PRINT_ANY, "arp_ignore", "arp_ignore %d ",
> >>> + conf[IPV4_DEVCONF_ARP_IGNORE - 1]);
> >>> +
> >>> + if (max_elements >= IPV4_DEVCONF_PROMOTE_SECONDARIES)
> >>> + print_string(PRINT_ANY, "promote_secondaries",
> >>> + "promote_secondaries %s ",
> >>> + conf[IPV4_DEVCONF_PROMOTE_SECONDARIES - 1] ?
> >>> "on" : "off");
> >>> +
> >>> + if (max_elements >= IPV4_DEVCONF_ARP_ACCEPT)
> >>> + print_int(PRINT_ANY, "arp_accept", "arp_accept %d ",
> >>> + conf[IPV4_DEVCONF_ARP_ACCEPT - 1]);
> >>> +
> >>> + if (max_elements >= IPV4_DEVCONF_ARP_NOTIFY)
> >>> + print_string(PRINT_ANY, "arp_notify", "arp_notify %s ",
> >>> + conf[IPV4_DEVCONF_ARP_NOTIFY - 1] ? "on" : "off");
> >>> +
> >>> + if (max_elements >= IPV4_DEVCONF_ACCEPT_LOCAL)
> >>> + print_string(PRINT_ANY, "accept_local", "accept_local %s ",
> >>> + conf[IPV4_DEVCONF_ACCEPT_LOCAL - 1] ? "on" :
> >>> "off");
> >>> +
> >>> + if (max_elements >= IPV4_DEVCONF_SRC_VMARK)
> >>> + print_string(PRINT_ANY, "src_vmark", " src_vmark %s",
> >>> + conf[IPV4_DEVCONF_SRC_VMARK - 1] ? "on" : "off");
> >>> +
> >>> + if (max_elements >= IPV4_DEVCONF_PROXY_ARP_PVLAN)
> >>> + print_string(PRINT_ANY, "proxy_arp_pvlan",
> >>> "proxy_arp_pvlan %s ",
> >>> + conf[IPV4_DEVCONF_PROXY_ARP_PVLAN - 1] ? "on" :
> >>> "off");
> >>> +
> >>> + if (max_elements >= IPV4_DEVCONF_ROUTE_LOCALNET)
> >>> + print_string(PRINT_ANY, "route_localnet",
> >>> "route_localnet %s ",
> >>> + conf[IPV4_DEVCONF_ROUTE_LOCALNET - 1] ? "on" :
> >>> "off");
> >>> +
> >>> + if (max_elements >= IPV4_DEVCONF_BC_FORWARDING)
> >>> + print_string(PRINT_ANY, "bc_forwarding", "bc_forwarding
> >>> %s ",
> >>> + conf[IPV4_DEVCONF_BC_FORWARDING - 1] ? "on" :
> >>> "off");
> >>> +
> >>> + if (max_elements >=
> >>> IPV4_DEVCONF_IGMPV2_UNSOLICITED_REPORT_INTERVAL)
> >>> + print_int(PRINT_ANY, "igmpv2_unsolicited_report_interval",
> >>> + "igmpv2_unsolicited_report_interval %d ",
> >>> +
> >>> conf[IPV4_DEVCONF_IGMPV2_UNSOLICITED_REPORT_INTERVAL - 1]);
> >>> +
> >>> + if (max_elements >=
> >>> IPV4_DEVCONF_IGMPV3_UNSOLICITED_REPORT_INTERVAL)
> >>> + print_int(PRINT_ANY, "igmpv3_unsolicited_report_interval",
> >>> + "igmpv3_unsolicited_report_interval %d ",
> >>> +
> >>> conf[IPV4_DEVCONF_IGMPV3_UNSOLICITED_REPORT_INTERVAL - 1]);
> >>> +
> >>> + if (max_elements >= IPV4_DEVCONF_IGNORE_ROUTES_WITH_LINKDOWN)
> >>> + print_string(PRINT_ANY, "ignore_routes_with_linkdown",
> >>> + "ignore_routes_with_linkdown %s ",
> >>> + conf[IPV4_DEVCONF_IGNORE_ROUTES_WITH_LINKDOWN -
> >>> 1] ?
> >>> + "on" : "off");
> >>> +
> >>> + if (max_elements >= IPV4_DEVCONF_DROP_UNICAST_IN_L2_MULTICAST)
> >>> + print_string(PRINT_ANY, "drop_unicast_in_l2_multicast",
> >>> + "drop_unicast_in_l2_multicast %s ",
> >>> + conf[IPV4_DEVCONF_DROP_UNICAST_IN_L2_MULTICAST
> >>> - 1] ?
> >>> + "on" : "off");
> >>> +
> >>> + if (max_elements >= IPV4_DEVCONF_DROP_GRATUITOUS_ARP)
> >>> + print_string(PRINT_ANY, "drop_gratuitous_arp",
> >>> + "drop_gratuitous_arp %s ",
> >>> + conf[IPV4_DEVCONF_DROP_GRATUITOUS_ARP - 1] ?
> >>> "on" : "off");
> >>> +
> >>> + if (max_elements >= IPV4_DEVCONF_ARP_EVICT_NOCARRIER)
> >>> + print_string(PRINT_ANY, "arp_evict_nocarrier",
> >>> + "arp_evict_nocarrier %s ",
> >>> + conf[IPV4_DEVCONF_ARP_EVICT_NOCARRIER - 1] ?
> >>> "on" : "off");
> >>> + }
> >>> +}
> >>> +
> >> There are three different ways to display a flag value in JSON used in
> >> iproute2.
> >> This one is my least favorite.
> >>
> >> The three ways are:
> >> - print_bool
> >> - print_null (only if on)
> >> - print_string
> >>
> >> I would use the print_null pattern but print_bool would also be ok.
> >>
> >
> > Thanks for the suggestion Stephen, I would pick print_bool in this case.
> >
> > If one of the options evolves to supporting something else we could
> > easily adapt it without breaking compatibility if we use print_bool. If
> > we use print_null I don't think we could do that.
> >
>
> Hm. I am actually not so sure about this..
>
> the current print_string approach matches the setter and also the
> netconf side. While print_bool would be easier to parse for JSON, it
> looks not so good for command line output.
>
> print_null presents a different problem, users would need to make sure
> their parsing is working with an iproute2 version that support these new
> attributes.
>
> So I am not so sure what is the best option here.
None of this is a hard requirement. The requirement is that the output
be valid JSON. The other common practices are:
- non JSON output of display should match the input command line
there were even some user tools that depended on this to do save/restore of state
- JSON output should be easy to parse in python.
If user is using python to read JSON (which seems like the most common).
then bool or presence allows for use in conditional.
>>> import json
>>> json.loads('{"forwarding": true}')["forwarding"]
True # <class 'bool'>
>>> json.loads('{"forwarding": "on"}')["forwarding"]
'on'
next prev parent reply other threads:[~2026-06-14 15:56 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-12 23:17 [PATCH iproute2-next] ipaddress: add support for showing IPv4 devconf attributes Fernando Fernandez Mancera
2026-06-13 2:29 ` Stephen Hemminger
2026-06-13 6:41 ` Fernando Fernandez Mancera
2026-06-13 7:22 ` Fernando Fernandez Mancera
2026-06-14 15:55 ` Stephen Hemminger [this message]
-- strict thread matches above, loose matches on Subject: below --
2026-06-12 23:27 Fernando Fernandez Mancera
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260614085556.1efda1eb@phoenix.local \
--to=stephen@networkplumber.org \
--cc=davem@davemloft.net \
--cc=dsahern@kernel.org \
--cc=edumazet@google.com \
--cc=fmancera@suse.de \
--cc=horms@kernel.org \
--cc=kuba@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox