On Tue, Mar 1, 2016 at 2:58 PM, Keller, Jacob E <jacob.e.kel...@intel.com> wrote: > On Tue, 2016-03-01 at 14:31 -0800, Alexander Duyck wrote: >> This still has the potential to provide garbage data. What you >> should >> probably do at each stage is make sure the length matches with the >> exact value that you would expect. >> > > Sure, an exact check could be done instead, however... > >> I assume you cannot have any fields shuffle on you? What I mean by >> that is that you don't want to have a setup with 4 Tx and 4 Rx rings >> where you then replace it with 1 Tx and 7 Rx rings and try to >> populate >> the same data into a setup where the strings reported are for 4 Tx >> and >> 4 Rx. You should double check that the length can be used as a means >> of identifying exactly what strings will be where. >> >> - Alex > > > Darn. Looks like you're right. It would be theoretically possible for > the number of queues (or other variables) to change such that the size > matches but the data no longer lines up against the strings. > > For queues, I don't think we're vulnerable on the fm10k driver, because > we only use combined queues. However, we already have support for > "debug-statistics" which shows extra stats plus some stats per virtual > function. I am not sure if these could change within the time window to > result in garbage data. > > I don't know how much of a real world problem this would be though. > > I'm guessing it's more reason to promote the idea of converting to some > new tool based on netlink.
Yeah, we had basically pushed the issue under the rug by using a static layout for statistics. Was the motivation for getting rid of the static layout just to declutter the display? I know I have gotten into the habit of just piping ethtool -S through a "| grep -v :\ 0" to drop any of the unused stats from the display. Also you may see issues depending on how the stats might be parsed by user-space scripts if you start modifying what is displayed and what is not. - Alex