netdev - RE: [RFC PATCH V2 net-next 5/5] ethtool: Add fallback to get_module

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 4 Mar 2021 18:44:37 -0800
From:   "Don Bollinger" <don@...bollingers.org>
To:     "'Andrew Lunn'" <andrew@...n.ch>
Cc:     "'Moshe Shemesh'" <moshe@...dia.com>,
        "'David S. Miller'" <davem@...emloft.net>,
        "'Jakub Kicinski'" <kuba@...nel.org>,
        "'Adrian Pop'" <pop.adrian61@...il.com>,
        "'Michal Kubecek'" <mkubecek@...e.cz>, <netdev@...r.kernel.org>,
        "'Vladyslav Tarasiuk'" <vladyslavt@...dia.com>,
        <don@...bollingers.org>
Subject: RE: [RFC PATCH V2 net-next 5/5] ethtool: Add fallback to get_module_eeprom from netlink command

> > > +static int fallback_set_params(struct eeprom_data_req_info *request,
> > > +			       struct ethtool_modinfo *modinfo,
> > > +			       struct ethtool_eeprom *eeprom) {
> >
> > This is translating the new data structure into the old.  Hence, I
> > assume we have i2c_addr, page, bank, offset, len to work with, and we
> > should use all of them.
> 
> Nope. We actually have none of them. The old API just asked the driver to
> give me the data in the SFP. And the driver gets to decide what it
returns,
> following a well known layout. The driver can decide to give just the
first 1/2
> page, or any number of multiple 1/2 pages in a well known linear way,
which
> ethtool knows how to decode.

This code is to take a new KAPI request (a struct eeprom_data_req_info), and
create an old driver API request (a struct ethtool_eeprom) that will get the

same data.  It isn't actually fetching the data, it is just forming the data
structure
to create the request.  So, we do indeed have all of the new KAPI
parameters, 
and need to use all of them to precisely create the matching old KAPI
request.

> So when mapping the new KAPI onto the old driver API, you need to call the
> old API, and see if what is returned can be used to fulfil the KAPI
request. If
> the bytes are there, great, return them, otherwise EOPNOTSUPP.

Actually, this code has to figure out in advance whether the old API can
return
the data to fulfill the request, then form a request to accomplish that.
 
> And we also need to consider the other way around. The old KAPI is used,
> and the MAC driver only supports the new driver API. Since the linear
layout
> is well know, you need to make a number of calls into the driver to read
the
> 1/2 pages, and them glue them together and return them.

That is a great idea, probably not difficult.  It is not in this patch set.
 
> I've not reviewed this code in detail yet, so i've no idea how it actually
works.
> But i would like to see as much compatibility as possible. That has been
the
> approach with moving from IOCTL to netlink with ethool. Everything the old
> KAPI can do, netlink should also be able to, plus there can be additional
> features.
> 
> > > +	switch (modinfo->type) {
> > > +	case ETH_MODULE_SFF_8079:
> > > +		if (request->page > 1)
> > > +			return -EINVAL;
> > > +		break;
> > > +	case ETH_MODULE_SFF_8472:
> > > +		if (request->page > 3)
> >
> > Not sure this is needed, there can be pages higher than 3.
> 
> Not with the old KAPI call. As far as i remember, it stops at three pages.
But i
> need to check the ethtool(1) sources to be sure.
> 
>        Andrew