netdev - Re: [PATCH ethtool-next v2] rxclass: Make output for RSS context action explicit

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] [day] [month] [year] [list]

Message-Id: <57047008-53c9-4744-b408-f78fef4c1871@app.fastmail.com>
Date: Tue, 12 Nov 2024 17:31:20 -0800
From: "Daniel Xu" <dxu@...uu.xyz>
To: "Jakub Kicinski" <kuba@...nel.org>
Cc: "Edward Cree" <ecree.xilinx@...il.com>, jdamato@...tly.com,
 "David Miller" <davem@...emloft.net>, mkubecek@...e.cz,
 "Martin KaFai Lau" <martin.lau@...ux.dev>, netdev@...r.kernel.org,
 "Kernel Team" <kernel-team@...a.com>
Subject: Re: [PATCH ethtool-next v2] rxclass: Make output for RSS context action
 explicit

Hi Jakub,

On Tue, Nov 12, 2024, at 7:40 AM, Jakub Kicinski wrote:
> On Mon, 11 Nov 2024 12:05:38 -0700 Daniel Xu wrote:
>> -	if (fsp->flow_type & FLOW_RSS)
>> -		fprintf(stdout, "\tRSS Context ID: %u\n", rss_context);
>> -
>>  	if (fsp->ring_cookie == RX_CLS_FLOW_DISC) {
>>  		fprintf(stdout, "\tAction: Drop\n");
>>  	} else if (fsp->ring_cookie == RX_CLS_FLOW_WAKE) {
>>  		fprintf(stdout, "\tAction: Wake-on-LAN\n");
>> +	} else if (fsp->flow_type & FLOW_RSS) {
>> +		u64 queue = ethtool_get_flow_spec_ring(fsp->ring_cookie);
>> +
>> +		fprintf(stdout, "\tAction: Direct to RSS context id %u", rss_context);
>
> Do you have strong feelings about the change in formatting?
> Looking at Ed's comment on the new test made me wonder if the change 
> in capitalization is for the better.
>
> Action: Direct to RSS context id 1 (queue base offset: 2)
>
> vs
>
> Action: Direct to RSS Context ID: 1 (queue base offset: 2)
>
> Given "id" is a word (: I like the ID format, the extra colon is
> annoying but OTOH if we retain it your regexp in the other patch
> would match before and after..
>
> Actually the best formatting IMHO would be to skip the ID altogether:
>
> Action: Direct to RSS Context 1 (queue base offset: 2)

No strong opinions other than I agree second colon should be skipped.
Let's go with this one.