Re: Broken link partner advertised reporting in ethtool

[Michal Kubecek]

On Mon, Jul 27, 2020 at 02:27:56PM -0700, Jacob Keller wrote:
> 
> 
> On 7/27/2020 2:08 PM, Michal Kubecek wrote:
> > On Mon, Jul 27, 2020 at 11:01:41PM +0200, Andrew Lunn wrote:
> >>>   - the exact command you ran (including arguments)
> >>>   - expected output (or at least the relevant part)
> >>>   - actual output (or at least the relevant part)
> >>>   - output with dump of netlink messages, you can get it by enabling
> >>>     debugging flags, e.g. "ethtool --debug 0x12 eth0"
> >>  
> >> Hi Michal
> >>
> >> See if this helps.
> >>
> >> This is a Marvel Ethernet switch port using an Marvell PHY.
> > 
> > Thank you. I think I can see the problem. Can you try the patch below?
> > 
> > Michal
> > 
> 
> I think the patch below fixes part of the issue, but isn't completely
> correct, because NOMASK bitmaps can be sent in compact form as well.

I believe this part is correct; compact NOMASK bitmap would have no
ETHTOOL_A_BITSET_MASK and ETHTOOL_A_BITSET_BIT_VALUE would contain the
bits in it so that the code would generate correct output.

> Also, we'll need something to check the NOMASK flag and do the correct
> thing in all of the dump functions.

You are right, bitset_get_bit needs the same fix, updated version is
below.

Michal


diff --git a/netlink/bitset.c b/netlink/bitset.c
index 130bcdb5b52c..67b45778692c 100644
--- a/netlink/bitset.c
+++ b/netlink/bitset.c
@@ -50,6 +50,7 @@ bool bitset_get_bit(const struct nlattr *bitset, bool mask, 
unsigned int idx,
        DECLARE_ATTR_TB_INFO(bitset_tb);
        const struct nlattr *bits;
        const struct nlattr *bit;
+       bool nomask;
        int ret;
 
        *retptr = 0;
@@ -68,6 +69,7 @@ bool bitset_get_bit(const struct nlattr *bitset, bool mask, 
unsigned int idx,
                return bitmap[idx / 32] & (1U << (idx % 32));
        }
 
+       nomask = bitset_tb[ETHTOOL_A_BITSET_NOMASK];
        bits = bitset_tb[ETHTOOL_A_BITSET_BITS];
        if (!bits)
                goto err;
@@ -87,7 +89,7 @@ bool bitset_get_bit(const struct nlattr *bitset, bool mask, 
unsigned int idx,
 
                my_idx = mnl_attr_get_u32(tb[ETHTOOL_A_BITSET_BIT_INDEX]);
                if (my_idx == idx)
-                       return mask || tb[ETHTOOL_A_BITSET_BIT_VALUE];
+                       return mask || nomask || tb[ETHTOOL_A_BITSET_BIT_VALUE];
        }
 
        return false;
diff --git a/netlink/settings.c b/netlink/settings.c
index 35ba2f5dd6d5..60523ad6edf5 100644
--- a/netlink/settings.c
+++ b/netlink/settings.c
@@ -280,6 +280,7 @@ int dump_link_modes(struct nl_context *nlctx, const struct 
nlattr *bitset,
        const struct nlattr *bit;
        bool first = true;
        int prev = -2;
+       bool nomask;
        int ret;
 
        ret = mnl_attr_parse_nested(bitset, attr_cb, &bitset_tb_info);
@@ -338,6 +339,7 @@ int dump_link_modes(struct nl_context *nlctx, const struct 
nlattr *bitset,
                goto after;
        }
 
+       nomask = bitset_tb[ETHTOOL_A_BITSET_NOMASK];
        printf("\t%s", before);
        mnl_attr_for_each_nested(bit, bits) {
                const struct nlattr *tb[ETHTOOL_A_BITSET_BIT_MAX + 1] = {};
@@ -354,7 +356,7 @@ int dump_link_modes(struct nl_context *nlctx, const struct 
nlattr *bitset,
                if (!tb[ETHTOOL_A_BITSET_BIT_INDEX] ||
                    !tb[ETHTOOL_A_BITSET_BIT_NAME])
                        goto err;
-               if (!mask && !tb[ETHTOOL_A_BITSET_BIT_VALUE])
+               if (!mask && !nomask && !tb[ETHTOOL_A_BITSET_BIT_VALUE])
                        continue;
 
                idx = mnl_attr_get_u32(tb[ETHTOOL_A_BITSET_BIT_INDEX]);