From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 70B02C43381 for ; Tue, 26 Feb 2019 17:24:31 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 48FDB21850 for ; Tue, 26 Feb 2019 17:24:31 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727823AbfBZRYa (ORCPT ); Tue, 26 Feb 2019 12:24:30 -0500 Received: from opengridcomputing.com ([72.48.214.68]:42416 "EHLO smtp.opengridcomputing.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727054AbfBZRY3 (ORCPT ); Tue, 26 Feb 2019 12:24:29 -0500 Received: from [10.10.0.239] (cody.ogc.int [10.10.0.239]) by smtp.opengridcomputing.com (Postfix) with ESMTPSA id 4EE2422605; Tue, 26 Feb 2019 11:24:29 -0600 (CST) Subject: Re: [PATCH v1 iproute2-next 3/4] rdma: add 'link add/delete' commands To: Leon Romanovsky Cc: dsahern@gmail.com, stephen@networkplumber.org, netdev@vger.kernel.org, linux-rdma@vger.kernel.org References: <5dd9d9aeada48bc5db0eb0394ed4e3ce38ee41bc.1550773362.git.swise@opengridcomputing.com> <20190223094311.GP23561@mtr-leonro.mtl.com> From: Steve Wise Message-ID: <94c6a44c-6f08-ffe8-9798-a4faf14bd29a@opengridcomputing.com> Date: Tue, 26 Feb 2019 11:24:32 -0600 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.5.1 MIME-Version: 1.0 In-Reply-To: <20190223094311.GP23561@mtr-leonro.mtl.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Content-Language: en-US Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On 2/23/2019 3:43 AM, Leon Romanovsky wrote: > On Thu, Feb 21, 2019 at 08:19:12AM -0800, Steve Wise wrote: >> Add new 'link' subcommand 'add' and 'delete' to allow binding a soft-rdma >> device to a netdev interface. >> >> EG: >> >> rdma link add rxe_eth0 type rxe netdev eth0 >> rdma link delete rxe_eth0 >> >> Signed-off-by: Steve Wise >> --- >> rdma/link.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ >> rdma/rdma.h | 1 + >> rdma/utils.c | 2 +- >> 3 files changed, 69 insertions(+), 1 deletion(-) >> >> diff --git a/rdma/link.c b/rdma/link.c >> index c064be627be2..afaf19663728 100644 >> --- a/rdma/link.c >> +++ b/rdma/link.c >> @@ -14,6 +14,9 @@ >> static int link_help(struct rd *rd) >> { >> pr_out("Usage: %s link show [DEV/PORT_INDEX]\n", rd->filename); >> + pr_out("Usage: %s link add NAME type TYPE netdev NETDEV\n", >> + rd->filename); >> + pr_out("Usage: %s link delete NAME\n", rd->filename); >> return 0; >> } >> >> @@ -341,10 +344,74 @@ static int link_show(struct rd *rd) >> return rd_exec_link(rd, link_one_show, true); >> } >> >> +static int link_add(struct rd *rd) >> +{ >> + char *name; >> + char *type = NULL; >> + char *dev = NULL; >> + uint32_t seq; >> + >> + if (rd_no_arg(rd)) { >> + pr_err("No link name was supplied\n"); > I think that it is better to have instruction message and not error > message: "Please provide ...". Ok.  Perhaps a new utility rd_exec_require_link() can be created? >> + return -EINVAL; >> + } >> + name = rd_argv(rd); >> + rd_arg_inc(rd); >> + while (!rd_no_arg(rd)) { >> + if (rd_argv_match(rd, "type")) { >> + rd_arg_inc(rd); >> + type = rd_argv(rd); >> + } else if (rd_argv_match(rd, "netdev")) { >> + rd_arg_inc(rd); >> + dev = rd_argv(rd); >> + } else { >> + pr_err("Invalid parameter %s\n", rd_argv(rd)); >> + return -EINVAL; >> + } >> + rd_arg_inc(rd); > Please use chains of struct rd_cmd and rd_exec_cmd() instead of > open-coding parser. Ok.  Like your recently merged series did... >> + } >> + if (!type) { >> + pr_err("No type was supplied\n"); >> + return -EINVAL; > General parser handle it. Ok. >> + } >> + if (!dev) { >> + pr_err("No net device was supplied\n"); >> + return -EINVAL; >> + } > rd_exec_require_dev() ??? Looks like I can use that.  I'll try it. Thanks! Steve.