From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id BB44F45C89; Tue, 5 Nov 2024 22:02:48 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 7C38D4027E; Tue, 5 Nov 2024 22:02:47 +0100 (CET) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mails.dpdk.org (Postfix) with ESMTP id 0817040156 for ; Tue, 5 Nov 2024 22:02:44 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1730840564; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=USq4bPQraYwZaI33Cj0En+VjW7TDFWf1FtH2Nkg5kmQ=; b=RL1PTYaKPcnO84EE7s01qSYMUSLFJm3YaylWv+xORRIXUZU6Cu5hvqkpakbUYuRVlcuu4s 3j326TkhKBQ6EZueBFZTMMBG/cFwumFI9tmdhZ1n+zPT66ZaT0rcUNCcOJHr9pgDoLSUhC 0D77OPF2S2LrWwaAuvJCLsMLAyQrS40= Received: from mail-lf1-f70.google.com (mail-lf1-f70.google.com [209.85.167.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-172-8C5JyayCPm-XQMC1YAnJIQ-1; Tue, 05 Nov 2024 16:02:38 -0500 X-MC-Unique: 8C5JyayCPm-XQMC1YAnJIQ-1 Received: by mail-lf1-f70.google.com with SMTP id 2adb3069b0e04-539e75025f9so3413754e87.3 for ; Tue, 05 Nov 2024 13:02:37 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1730840557; x=1731445357; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=USq4bPQraYwZaI33Cj0En+VjW7TDFWf1FtH2Nkg5kmQ=; b=hJauFuISFWBXVudfM0zLNHIkA5c5ruD7Jsp92KrzPaBTfGfdIe+JvIh/y2dRTO22LO pRyWoBxrvQgpcVHx0GOPHK3iF6Oa9FaTc6W7ENhyCR1BX/y+pRhOacNEi8SznHUVhlSi gIUV3MpHtpheg7CrPsxqG4LqnZt06Jz8nPdbTOoY6BA293yf5wGwz6BYmfRXhn0Qho4l 8N88o4s/34pXHFUogjQTbfBO2JR1m2YhrdBMCK/NzzwmowKR5era0CX8cxtQeBkpsLyH FcQwnPsSHDwj6Ske3ARAO31rHM8klr4w6Xrlk5+ZZo+2ZZvVcr2FUJOzvFO6+1XQG693 Ddfw== X-Forwarded-Encrypted: i=1; AJvYcCVDAPlJlgcMb8IZfosd9IGcmJ2C5NgsZR6nuR10JFdVY/kI0uEmN9tS15p1LaRumFVlY4c=@dpdk.org X-Gm-Message-State: AOJu0Yyzr72UTOjjsMKxdyOFbccIo89RbCzJ5ZBZ9s06OhYUkceW4KTh sZFQWRT7ka7SPCi4JORKyexgbZtOHGnlPLj5cpV9ML70WXAQswC6evjA/B3w0DqL7sKEjnLb6N0 t9xH+0+rGkMeC08yzSRiUrzNZfyTyCghlPjZgeIagoPE62ZttO1LwL4ruOYW75wnl/JDK8w7NEk 9c6LGVUQkaIrJBHy4= X-Received: by 2002:a05:651c:544:b0:2fb:61d9:d72b with SMTP id 38308e7fff4ca-2fcbdf5a207mr176270921fa.1.1730840556598; Tue, 05 Nov 2024 13:02:36 -0800 (PST) X-Google-Smtp-Source: AGHT+IEqQ3zotokD9bcUcW+BlXHKeO5/IzCL5vyVTw7Obpk6Z+eG61O1wv6jl/5su961xyA4RzcIUgbLWJDisbkOt7Y= X-Received: by 2002:a05:651c:544:b0:2fb:61d9:d72b with SMTP id 38308e7fff4ca-2fcbdf5a207mr176270711fa.1.1730840556100; Tue, 05 Nov 2024 13:02:36 -0800 (PST) MIME-Version: 1.0 References: <20241030040109.34504-1-fengchengwen@huawei.com> In-Reply-To: <20241030040109.34504-1-fengchengwen@huawei.com> From: David Marchand Date: Tue, 5 Nov 2024 22:02:24 +0100 Message-ID: Subject: Re: [PATCH] dpdk: remove redundant null check when parse kvargs To: Chengwen Feng Cc: thomas@monjalon.net, ferruh.yigit@amd.com, dev@dpdk.org, Gagandeep Singh , Hemant Agrawal , Nicolas Chautru , Rosen Xu , Kai Ji , Kevin Laatz , Bruce Richardson , Abdullah Sevincer , Srikanth Yalavarthi , Ajit Khaparde , Somnath Kotur , Chas Williams , "Min Hu (Connor)" , Gaetan Rivet , Jie Hai , Vladimir Medvedkin , Ian Stokes , Anatoly Burakov , Chaoyong He , Christian Koue Muf , Serhii Iliushyk , Tetsuya Mukawa , Cristian Dumitrescu , Stephen Hemminger , Jiawen Wu , Jian Wang , Maxime Coquelin , Chenbo Xia , Sachin Saxena , Vijay Kumar Srivastava , Fan Zhang , Ashish Gupta , Akhil Goyal , Andrew Rybchenko X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Hello, On Wed, Oct 30, 2024 at 5:00=E2=80=AFAM Chengwen Feng wrote: > diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.= c > index 36b06b3ac5..9366cb39b8 100644 > --- a/drivers/net/tap/rte_eth_tap.c > +++ b/drivers/net/tap/rte_eth_tap.c > @@ -2484,19 +2484,19 @@ rte_pmd_tap_probe(struct rte_vdev_device *dev) > } > > if (rte_kvargs_count(kvlist, ETH_TAP_REMOTE_ARG) = =3D=3D 1) { > - ret =3D rte_kvargs_process(kvlist, > - ETH_TAP_REMOTE_A= RG, > - &set_remote_ifac= e, > - remote_iface); > + ret =3D rte_kvargs_process_opt(kvlist, > + ETH_TAP_REMO= TE_ARG, > + &set_remote_= iface, > + remote_iface= ); > if (ret =3D=3D -1) > goto leave; > } > > if (rte_kvargs_count(kvlist, ETH_TAP_MAC_ARG) =3D= =3D 1) { > - ret =3D rte_kvargs_process(kvlist, > - ETH_TAP_MAC_ARG, > - &set_mac_type, > - &user_mac); > + ret =3D rte_kvargs_process_opt(kvlist, > + ETH_TAP_MAC_= ARG, > + &set_mac_typ= e, > + &user_mac); > if (ret =3D=3D -1) > goto leave; > } Ok, it restores compatibility with some strange arguments passed by user (and, on the principle, for this reason, it should be in a separate commit with a Fixes: de89988365a7 ("kvargs: rework process API")). But on the other hand, this case does not make sense. Those two helpers do nothing if there is no value. Plus, those arguments are documented as taking a value: ETH_TAP_MAC_ARG "=3D" ETH_TAP_MAC_ARG_FMT " " ETH_TAP_REMOTE_ARG "=3D"); So in the end, the current logic looks more like an oversight in the code, and the case "only key" should be rightfully rejected. IOW: I would keep rte_kvargs_process() for them, but remove checks on value =3D=3D NULL in those two helpers. Extract: static int set_mac_type(const char *key __rte_unused, const char *value, void *extra_args) { struct rte_ether_addr *user_mac =3D extra_args; if (!value) return 0; ... } static int set_remote_iface(const char *key __rte_unused, const char *value, void *extra_args) { char *name =3D (char *)extra_args; if (value) { ... } return 0; } The rest of the patch looks correct to me, and I did not spot a missed upda= te. Thanks. --=20 David Marchand