On 27/04/16 12:03, Andrew Lunn wrote:
>> + if (stringset == ETH_SS_STATS && ds->drv->get_strings) {
>> + ndata = data + mcount * len;
>> + /* This function copies ETH_GSTRINGS_LEN bytes, we will mangle
>> + * the output after to prepend our CPU port prefix we
>> + * constructed earlier
>> + */
>> + ds->drv->get_strings(ds, cpu_port, ndata);
>> + count = ds->drv->get_sset_count(ds);
>> + for (i = 0; i < count; i++) {
>> + memmove(ndata + (i * len + sizeof(pfx)),
>> + ndata + i * len, len - sizeof(pfx));
>> + memcpy(ndata + i * len, pfx, sizeof(pfx));
>
> Hi Florian
>
> Did you check what happens if this causes the NULL terminator to be
> discarded? Does ethtool handle that? As i said before, it is unclear
> if one is required.
I just did yes. So ethtool has a do_gstringset() function which
NULL-terminates every strings set except the statistics kind
(ETH_SS_STATS or ETH_SS_PHY_STATS) but this is not much of a problem
because it limits the output to ETH_GSTRING_LEN anyway.
After injecting a bit of error in net/dsa/slave.c to have a much bigger
prefix making us push the stats names, the stats are correcty truncated
by ethtool. So we seem to be good to go with the current code in kernel
and user space.
--
Florian
