Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Subject: Re: [PATCH net-next v4 2/4] net: tap: track dropped skb via
 kfree_skb_reason()
To:     Jakub Kicinski <kuba@kernel.org>
Cc:     netdev@vger.kernel.org, bpf@vger.kernel.org,
        linux-kernel@vger.kernel.org, davem@davemloft.net,
        rostedt@goodmis.org, mingo@redhat.com, ast@kernel.org,
        daniel@iogearbox.net, andrii@kernel.org, imagedong@tencent.com,
        joao.m.martins@oracle.com, joe.jin@oracle.com, dsahern@gmail.com,
        edumazet@google.com
References: <20220226084929.6417-1-dongli.zhang@oracle.com>
 <20220226084929.6417-3-dongli.zhang@oracle.com>
 <20220301184209.1f11b350@kicinski-fedora-PC1C0HJN.hsd1.ca.comcast.net>
From:   Dongli Zhang <dongli.zhang@oracle.com>
Message-ID: <0556b706-cb4d-b0b6-ef29-443123afd71d@oracle.com>
Date:   Wed, 2 Mar 2022 09:43:29 -0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.6.1
In-Reply-To: <20220301184209.1f11b350@kicinski-fedora-PC1C0HJN.hsd1.ca.comcast.net>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?UDVQM0J0SFUvOVUrN3Y5Z0taTVdQY1pLS2VIL295N0MxL3JFNFJvZ2NLRzZI?=
 =?utf-8?B?NE1IdUxVZWsvcitnM29FSldRa3FnR0swL2F2eXVSWmpIaS9SaWlHbEtjRHo0?=
 =?utf-8?B?bjh4NWRGWjZSczMzWXd3blcydzZKbE5qZUhhMmRVNFhBY2xNZlB0NGlBbS8x?=
 =?utf-8?B?cC9hSXd6SngvSlltRHZXSmZZaCtrN3o0cVVFL2EvSk1wRW1qbEhUZjdNSlBW?=
 =?utf-8?B?MCtaeGtmajdUTVlmTTg5WkNQWWMvTklyVC91V0N4aDl2K01iLzhtVGczZi81?=
 =?utf-8?B?SXJFMXlIblpVdXVKc3dnWEJQZmFjcU5aRVRLZWxrZ05rOG9SaVdmWU5tVFpa?=
 =?utf-8?B?U1lJM3BwRDYrT21TeU1YK3Q1Y3BmMTNzdWNxMzN2LzJmOXIyTnRhYjFKa3NU?=
 =?utf-8?B?K0liaS9nZUhQZUlTeUd1bUJTdEZtNUZpUnJlZDVielBvc0tkT21Ec2pSbzJO?=
 =?utf-8?B?eVNYTTAxWllqL0Q5Y3g2WkcweTF6WjhwLzZvRUpVTDd4dm5pV21xQnRWR2Rq?=
 =?utf-8?B?ZUlzbldWVWdwSTA5bjRJOGhWR2Y5TFFRY3VBc0JzQ0FVeXBwbkJ6QXkvVldy?=
 =?utf-8?B?dGZQMmhvdTQ2RnhybTlGZjZzUDErVzhpL2thSTFoVXF2eGM3Z0NtSndVNVdu?=
 =?utf-8?B?SDBKbHZTcGFxdXBrNEU4WFptYmd3ZDVCbFBzT3g5T04zMythSzlTS1dKYmo5?=
 =?utf-8?B?VS9jUzk0WG43aDhyMkpOVzdUbXR1UGJOdmVmempIQm9TWmF3TTV6RDhlc1RZ?=
 =?utf-8?B?aGRlZmFpTXB1QkwwT0M0YWtyLzczV0VQdVZ3d2RSR2E5bWdycU5vN1MrNWp3?=
 =?utf-8?B?RFB0MkU0OEk1YjNZaHNyRzlraEVEb3p4TFRpS080YUQzUGU2V24xK2VuZG5a?=
 =?utf-8?B?ekd5OHA5RXArYWhwMkFLR281L2VXSWw3NCtVR1J4MDI3b0JXakNoSjZMWWEx?=
 =?utf-8?B?OS91T3lZRUVvTVVzUHBxZzBrK3dBSFRmMk1OU04zUkNPc2tiN1hOMFUyWEti?=
 =?utf-8?B?ZzhaWk1ubTBJUWw2VW5aa09Mdis1TC95OG50NWdJUUVvM0hkdHdGYnlGcVg4?=
 =?utf-8?B?NFI4d1JvanRLZlNaUEpkdkVXekNpLzVtYytaQXBBRTdvYXFMNlZTN3lZVkFO?=
 =?utf-8?B?eW5qTzkzZ0xSZndHMEZFbUU3VXVCNUQ0RVJVY0lmTTltdmhvME1NSDBxTy9D?=
 =?utf-8?B?MlNpWGd1RklPT3VxZ0RoZWFLcUc5cEhwSDdRa0xzTFViL281RW5UbE1zQTNh?=
 =?utf-8?B?NmJaQ1lLcDZZQ0RmTXNyK1YrV2pjSGFFa3pWdHllajBvWHFnK1FNd21zZ0ZW?=
 =?utf-8?B?cWZ3bEtaUHZMeU1kMnpYMmk0aVFnNlRoN2l4azQ4Y2NQTno5U1BvUkpaNWds?=
 =?utf-8?B?aklFNWtBQ3U4bGZGOXpseGFUdmQ2QTFiN3FWZHJ6b1NnSDlKWTV1cmFJRWpi?=
 =?utf-8?B?WGp4OTlxcllienMyTGRtQlR4S2xTTU0zSEt4dkpFOXo4UnVRQjRCUWJOUmY4?=
 =?utf-8?B?YzNGem9KZW9zWXg3RUJsZDB3YzVIOHRjYzBlSzdVdHg0NkFuSVE0MFZNelg0?=
 =?utf-8?B?K2JxS2NUN3Q5c0pYbkJsazVxZktpQzFwQVRVcUJvZ29Gai9JYXMrR29QblJn?=
 =?utf-8?B?ZCtnVHFJTXNGdzBndDdqVUhkOWxNaHlDbVZLVVN1VXRBZHk2MUk2MGFObVhH?=
 =?utf-8?B?UkRXbzVnRXN1d2FWZ1FvSUlPUHE3V0daYXRydlhPUDFDSWprTjB2V3ZrdTQv?=
 =?utf-8?B?bmNsbTFyY0xpdXFkajRncG1MOFJRQVJncFRlUHJQdFVZZWc5b2xGZThUZVZX?=
 =?utf-8?B?ZVE0KzFwc25WNjhzdVdmVmhpU2oycWx5VjBuK0xPNEhXSlF2cUxjNlRGUFgv?=
 =?utf-8?B?SDdSUGVpdUFPZU5DWEl4ZVdwb2J0YmR6QlU5Ykt3T210V0RTWFl2SE42L0hP?=
 =?utf-8?B?Vzh2VTVqVk4zZ05jcjFiY08xa1F4ZW1pNEtOeWFXOEZxakZnWHFjM2NVQ0tD?=
 =?utf-8?B?ZmdFSlgyR1pMSmhHaFZxeEhuWGN4TWF2N294MndvWjkzRG5LbnAwQk9ZY0xy?=
 =?utf-8?B?a09BK0NkWDRreVVuOFl3YmRjNGhYRHcyeGMyQ096U240TXU3dDRlZEx3WXNV?=
 =?utf-8?B?N3REUHhuRXpEeTY3bU0waFR1YXJBNzhrYThqZUw4M0tEaDl1M0RnSDN0UXpS?=
 =?utf-8?B?elI1SzVRVC8wSEtKQjR6ODIya3lSSFZ2Yk56R3lxY1B4TkhLR04rc0cyQ2wr?=
 =?utf-8?B?Mmp4aTdyQ2k3K2FhRGxKemlpMWx3PT0=?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: d4c2d14a-15fe-4e19-e2c5-08d9fc742a8f
X-MS-Exchange-CrossTenant-AuthSource: BYAPR10MB2663.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 02 Mar 2022 17:43:31.1833
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: o28aE79pnboI0Oi9DOjtmPowZp8mUZ2i9Jqh6xU3PK2JoQosLng5KN4JxUNRh2VNTZcMVA7mLbb5Z3b1VBZSRw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS7PR10MB5022
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10274 signatures=686787
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 malwarescore=0 mlxscore=0 phishscore=0
 bulkscore=0 adultscore=0 spamscore=0 suspectscore=0 mlxlogscore=920
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2201110000
 definitions=main-2203020077
X-Proofpoint-GUID: ZKUJXPgL-QsShNPWF89ddfKL9eYh71rZ
X-Proofpoint-ORIG-GUID: ZKUJXPgL-QsShNPWF89ddfKL9eYh71rZ
X-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,NICE_REPLY_A,RDNS_NONE,SPF_HELO_NONE,
	T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
	lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Jakub,

On 3/1/22 6:42 PM, Jakub Kicinski wrote:
> On Sat, 26 Feb 2022 00:49:27 -0800 Dongli Zhang wrote:
>> +	SKB_DROP_REASON_SKB_CSUM,	/* sk_buff checksum error */
> 
> Can we spell it out a little more? It sounds like the checksum was
> incorrect. Will it be clear that computing the checksum failed, rather
> than checksum validation failed?

I am just trying to make the reasons as generic as possible so that:

1. We may minimize the number of reasons.

2. People may re-use the same reason for all CSUM related issue.

> 
>> +	SKB_DROP_REASON_SKB_COPY_DATA,	/* failed to copy data from or to
>> +					 * sk_buff
>> +					 */
> 
> Here should we specify that it's copying from user space?

Same as above. I am minimizing the number of reasons so that any memory copy for
sk_buff may re-use this reason.

Please let me know if you think it is very significant to specialize the usage
of reason. I will then add "sk_buff csum computation failed" and "userspace".

> 
>> +	SKB_DROP_REASON_SKB_GSO_SEG,	/* gso segmentation error */
>> +	SKB_DROP_REASON_DEV_HDR,	/* there is something wrong with
>> +					 * device driver specific header
>> +					 */
> 
> How about:
> device driver specific header / metadata was invalid
> 
> to broaden the scope also to devices which don't transfer the metadata
> in form of a header?

I will add 'metadata'.

Thank you very much!

Dongli Zhang

> 
>> +	SKB_DROP_REASON_FULL_RING,	/* ring buffer is full */