Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.35 as permitted sender) client-ip=23.128.96.35;
Message-ID: <0746c819-b44d-4096-81f4-8946e2f5f7a2@intel.com>
Date:   Fri, 27 Oct 2023 09:57:07 -0700
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH net-next v2 3/5] net: phy: Add pluming for
 ethtool_{get,set}_rxnfc
Content-Language: en-US
To:     Florian Fainelli <florian.fainelli@broadcom.com>,
        <netdev@vger.kernel.org>
CC:     Doug Berger <opendmb@gmail.com>,
        Broadcom internal kernel review list 
        <bcm-kernel-feedback-list@broadcom.com>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>, Andrew Lunn <andrew@lunn.ch>,
        Heiner Kallweit <hkallweit1@gmail.com>,
        Russell King <linux@armlinux.org.uk>,
        Vladimir Oltean <vladimir.oltean@nxp.com>,
        "Tariq Toukan" <tariqt@nvidia.com>, Gal Pressman <gal@nvidia.com>,
        Willem de Bruijn <willemb@google.com>,
        Daniil Tatianin <d-tatianin@yandex-team.ru>,
        "Simon Horman" <horms@kernel.org>,
        Justin Chen <justin.chen@broadcom.com>,
        "Ratheesh Kannoth" <rkannoth@marvell.com>,
        Joe Damato <jdamato@fastly.com>,
        "Vincent Mailhol" <mailhol.vincent@wanadoo.fr>,
        Jiri Pirko <jiri@resnulli.us>,
        "open list" <linux-kernel@vger.kernel.org>
References: <20231026224509.112353-1-florian.fainelli@broadcom.com>
 <20231026224509.112353-4-florian.fainelli@broadcom.com>
 <0e64566f-6dfe-49a7-aeac-21ed9e66bd75@intel.com>
 <6bf05528-0d82-4b7b-b584-5c1d32b25ea0@broadcom.com>
From:   Jacob Keller <jacob.e.keller@intel.com>
In-Reply-To: <6bf05528-0d82-4b7b-b584-5c1d32b25ea0@broadcom.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?Ym5meXpzc1ZNNldOSmhCYlBsdFREVm1NTmU0MHZDNG83Ymg5MWhIN1UwRERY?=
 =?utf-8?B?YlF4cTdBQ3pwQk8yaGlteWFYTUY4OEVRcHVNaGNETUN2Wk5ocEROU1FxQlpr?=
 =?utf-8?B?Q0dkUkd5a2s2VzBmdWMrTXZ1TEgwdWVuZVdxQ3FMKzZyd0x4bTVlTnR3MFAx?=
 =?utf-8?B?amlqTXhFMUJxNzFGTWE5a3V2SzZhaE9ucjhha3NLTENDN3d2dTAyeUxSK3Ux?=
 =?utf-8?B?R2liOTlseU1aazZ4dGVsM01mVnRZMkkyT2RWNXpPdUMvZ1Q2MEtvellLd1lw?=
 =?utf-8?B?REFPdERJNGhya2Jxc1I2bEdheFowMDRvNDM5Q0JoR3B6TlV3UVIvdVIvYnI0?=
 =?utf-8?B?RE5QNmd4YzhWaThsTndKY2k0Q3VXODV5eXliZ01XSzJ1MG9pdGhmQmpONVVM?=
 =?utf-8?B?blpycEVReGllb29OZWk1TFhvNUU2R2dLem5KZHhTMUVZL3NwY3pEZGRTaG1Z?=
 =?utf-8?B?SkNWNVF2czRHUlpSWFEyWTNTbTNzZlMrSitBc29BSUFwMlJ4Rzk5eHFFMWZs?=
 =?utf-8?B?U21JMC9yZHE3aWJtZ1BoNVYxT25PcmNzT2Y5NUdOT0svYnBKN3YzdUh1UEIz?=
 =?utf-8?B?a2s0Q0RiT3VTMlc5eEJDRDhGNTk1cHVwRktqbmg4TmVkZHE1eWZrM1dBdTMr?=
 =?utf-8?B?alZhMzFRc0pvUy8vTkNvdjdhVHN0SXpnN3YwYk8zRWxBSHlDcUZ6Q3pnMWZ2?=
 =?utf-8?B?VVl6WWNFcjFBa09rQllGRERLOUZtRFdLa3lpYUlFNGNBeUVpbWVtZ29LSGFy?=
 =?utf-8?B?cm1ySGZhcW8vY0RQbFl4MHAzWWk4SGtZWUFxbVBtRStNT2NMei9Ya2M5di8w?=
 =?utf-8?B?L3JLM1BhYnduei9UdU9uaHd1dXBpbzVsQjE4czZnc2RYcW4wcDRGb1lyTkdI?=
 =?utf-8?B?bkxOVzlWSGZtdk9id2QyU3dVVlMrcHMxVWVHeGRnamRwTTZOZkRHWElSUG5T?=
 =?utf-8?B?b2U0TjlwT2ZEZ1dFRDBncjNEN2dJZGVFditnZTVHVm5zUjFXVzJaSjZZeUlk?=
 =?utf-8?B?M05NSEVlek5Yd2pxZk5xZUtING5LTUROeTZxU3FXZmFDSmY2TW9jamViTmJ0?=
 =?utf-8?B?NS9wdFViOXRIa0QraFNCQzJDOWI4OTE4OS9VNG1odEdBTEdzVnZaaWtud1lM?=
 =?utf-8?B?TC9HSnNkNG5va2MxSVZqdDZpY2t5bWFqaUtRL3NFZ3ovaGZjRTJ2ZWZRdFR1?=
 =?utf-8?B?Rk1iVUJVeWw1ajJ1Zzl3Q1F5YzBzS3lISStGdVhLdG0wSU1VZGJWUkhyTEpL?=
 =?utf-8?B?eGlzdllIdEdyVHd0RUszNzVRNzZBNDRUTEh1TkFvdFRTRzdocVB0ZTJjWmgz?=
 =?utf-8?B?MFJmaHl4WWg2UmU3eHByYzBRQUg1TFF4NjJDQitQYStweFg0UTVyTnRjVHFu?=
 =?utf-8?B?MUMrRFBHY3FqaW4xL3BZdElidjhINlp6YjVWWktZaUxqYm1MVFFmbGtnVFB3?=
 =?utf-8?B?RW9uZVpkQnJNMk1IOGdYS3lHYklVWUpKNXJCd3lhR1RGWlo1QUEwa1JkMEZx?=
 =?utf-8?B?Nm9TenB1QXFDMjVOV0FjUmtlMzFQZWpFNUd1UUgwYjlBYStObzJmMGRvb0o3?=
 =?utf-8?B?U1E5UEZjMnpka0o2Q3BBN0lBNzV3VTRPdnpXKzVUU1o3OXZ0M3hTT2c3b1BT?=
 =?utf-8?B?TkoyQ3JpY2VuVWVIOWZTek5jWXhhLzB6M1U4cXhTbExENVJWa29mUWdBMzdt?=
 =?utf-8?B?VFgzT0JPSGN0aTV0OTRKYXg0VTM2dllkaVAvMDF0UkdSQWl4cWt1enJwZXZy?=
 =?utf-8?B?Y2JKMWFaa0dRUDdUYTlOU3BBVkx0OUJ3alM3VXFyWEttL3JmajNob25mcmEx?=
 =?utf-8?B?elVXQVZJbzVnRGxXSWZNVGJNUWhMUW91QmhKQ3FNWDNTdk9oNzZubXFmSzJu?=
 =?utf-8?B?TUJia2FNM1BQMnJsRlNwN2JLUWlFZEcyWXd6RUI3ZXR1VmE2VzdjWnZ4aE5T?=
 =?utf-8?B?aGdJL1k4bTV2UE5IWmFzblVuaFdkUHptN1BtZEdBa1N3TWhwWVA2KzM0TkhH?=
 =?utf-8?B?ZDVPR1Q1SHAvN2lrQVZjbmJQekVCcDl1cFd5M1IreVBzQkF2WFZ5Vm54Ty8w?=
 =?utf-8?B?bkZZd0MwRU5CMlVsUjBMR0MvdEpmb2hZWG5BRkZqVjlxWGs5T1M0Y1JPZ3lv?=
 =?utf-8?B?ZGNjK3EwQjFRODNxRXFWYWlneVIyRXVQcEdDTWwzTlVRb1BCUlZWZ2FPUitW?=
 =?utf-8?B?REE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 277a6bda-dbc6-4ab6-ad12-08dbd70dc25a
X-MS-Exchange-CrossTenant-AuthSource: CO1PR11MB5089.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 27 Oct 2023 16:57:09.9187
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: nkewSfGvUCsJQ1u6dbwWMhK6JpsgeREvIKDGRR64Zf5gN+qEHvSoLPAfFoajrgGmUV/n8JxBfZw5xyL997kr0yUX9D+KILd4s5R9Y3fT2L4=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR11MB7657
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable
	autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on groat.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (groat.vger.email [0.0.0.0]); Fri, 27 Oct 2023 09:57:25 -0700 (PDT)


On 10/26/2023 4:32 PM, Florian Fainelli wrote:
> On 10/26/23 16:20, Jacob Keller wrote:
>>
>>
>> On 10/26/2023 3:45 PM, Florian Fainelli wrote:
>>> +EXPORT_SYMBOL(phy_ethtool_set_rxnfc);
>>> diff --git a/include/linux/phy.h b/include/linux/phy.h
>>> index 3cc52826f18e..03e7c6352aef 100644
>>> --- a/include/linux/phy.h
>>> +++ b/include/linux/phy.h
>>> @@ -1077,6 +1077,10 @@ struct phy_driver {
>>>   	int (*get_sqi)(struct phy_device *dev);
>>>   	/** @get_sqi_max: Get the maximum signal quality indication */
>>>   	int (*get_sqi_max)(struct phy_device *dev);
>>> +	/* Used for WAKE_FILTER programming only */
>>
>> Any particular reason this comment is required? I don't see it enforced
>> above so I'm curious.
> 
> The comment is not required, though I put it in there to help readers 
> understand that this is purely for use by Wake-on-LAN, since unlike a 
> MACs, PHYs are not capable of redirecting certain flows to certain 
> queues, too low in the packet processing that there is not a notion of a 
> queue at that point (FIFO yes, but not queue as in what the networking 
> stack wants to use).

Ah, right that makes sense. Thanks for the clarification.

Perhaps we could enforce that in the interface so that if something does
try to pass a different kind of filter to the phy_rxnfc it would fail or
complain somehow? Because that seems like it would be a programming mistake.

I guess with the current implementation you effectively rely on
returning -EOPNOTSUP for non-wake filters since the driver forwards the
filter to the PHY to check whether it can support it.

Thanks,
Jake