On Tue, Jul 28, 2020 at 07:09:17PM +0200, Klemens Nanni wrote:
> bridge_status() and switch_status() do the regular sanity check with
> SIOCGIFFLAGS, but both functions also call is_switch(), bridge_status()
> also calls is_bridge().
>
> Those is_*() helpers do the same SIOCGIFFLAGS sanity check, making those
> in *_status() entirely redundant, so I'd like to remove them.
Small correction: is_bridge() does SIOCGIFFLAGS, is_switch() does not.
Below is a new diff that removes the SIOCGIFFLAGS check form is_bridge()
as well, leaving the two is_*() helpers to their driver specific ioctls
alone.
> I'm here since the tpmr(4) ioctl interface transition from trunk to
> bridge semantics is now complete, so ifconfig(8) now requires tpmr bits
> to show its members in bridge fashion.
>
> One way would be duplicate code into is_tpmr() and tpmr_status() which
> I've already done, but another approach is to unify all bridge like
> interfaces under bridge_status().
With this in, merging switch_status() into bridge_status() is a trivial
diff, adding tpmr awareness to the mix would then be another diff after
that.
> Either ways, diff below cleans up and makes for simpler code.
So this effectively just removes SIOCGIFFLAGS in brconfig.c which are of
no use, imho. ifconfig.c:getinfo() already checks interfaces flags,
even more than once, for all interfaces.
> Feedback? OK?
Index: brconfig.c
===================================================================
RCS file: /cvs/src/sbin/ifconfig/brconfig.c,v
retrieving revision 1.25
diff -u -p -r1.25 brconfig.c
--- brconfig.c 22 Jan 2020 06:24:07 -0000 1.25
+++ brconfig.c 29 Jul 2020 00:58:40 -0000
@@ -762,14 +762,8 @@ bridge_holdcnt(const char *value, int d)
int
is_bridge()
{
- struct ifreq ifr;
struct ifbaconf ifbac;
- strlcpy(ifr.ifr_name, ifname, sizeof(ifr.ifr_name));
-
- if (ioctl(sock, SIOCGIFFLAGS, (caddr_t)&ifr) == -1)
- return (0);
-
ifbac.ifbac_len = 0;
strlcpy(ifbac.ifbac_name, ifname, sizeof(ifbac.ifbac_name));
if (ioctl(sock, SIOCBRDGRTS, (caddr_t)&ifbac) == -1) {
@@ -783,16 +777,11 @@ is_bridge()
void
bridge_status(void)
{
- struct ifreq ifr;
struct ifbrparam bp1, bp2;
if (!is_bridge() || is_switch())
return;
- strlcpy(ifr.ifr_name, ifname, sizeof(ifr.ifr_name));
- if (ioctl(sock, SIOCGIFFLAGS, (caddr_t)&ifr) == -1)
- return;
-
bridge_cfg("\t");
bridge_list("\t");
@@ -1184,13 +1173,7 @@ switch_cfg(char *delim)
void
switch_status(void)
{
- struct ifreq ifr;
-
if (!is_switch())
- return;
-
- strlcpy(ifr.ifr_name, ifname, sizeof(ifr.ifr_name));
- if (ioctl(sock, SIOCGIFFLAGS, (caddr_t)&ifr) == -1)
return;
switch_cfg("\t");
