public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
From: Stephen Hemminger <stephen@networkplumber.org>
To: Matt Ellison <matt@arroyo.io>
Cc: netdev@vger.kernel.org
Subject: Re: [PATCH iproute2] ip: support for xfrm interfaces
Date: Mon, 31 Dec 2018 22:11:22 -0800	[thread overview]
Message-ID: <20181231221122.1829f0f1@shemminger-XPS-13-9360> (raw)
In-Reply-To: <20181228103211.2023498c@aquamarine>

On Fri, 28 Dec 2018 10:32:11 -0500
Matt Ellison <matt@arroyo.io> wrote:

> Support for new (4.19+) xfrm virtual interfaces.
> 
> Interfaces take a 'if_id' which is an interface id which can be set on
> an xfrm policy as its interface lookup key (XFRMA_IF_ID).
> 
> Signed-off-by: Matt Ellison <matt@arroyo.io>

Wanted to apply this, but it has lots of style issues.
The biggest one is indenting with spaces instead of tabs.
You can use the kernel checkpatch utility to ensure that it is correct.

Other changes needed:
1. For new code just use SPDX style license id, no need to quote GPL
/* SPDX-License-Identifier: GPL-2.0 */

2. Please make help function simpler:

static void xfrm_print_help(struct link_util *lu,
			    int argc, char **argv, FILE *f)
{
	fprintf(f, "Usage: ... %-4s dev PHYS_DEV [ if_id IF-ID ]\n", lu->id);
	fprintf(f, "\nWhere: IF-ID := { 0x0..0xffffffff }\n");
}

The code part looks ok, but you should also update the man page
and add test cases if possible.

Please fix and resubmit.

  reply	other threads:[~2019-01-01  6:11 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-28 15:32 [PATCH iproute2] ip: support for xfrm interfaces Matt Ellison
2019-01-01  6:11 ` Stephen Hemminger [this message]
2019-01-01 17:40   ` Matt Ellison
2019-04-04 16:25     ` [iproute2] " Antony Antony
2019-04-05 19:46       ` Matt Ellison
2019-04-05 21:03         ` Antony Antony
2019-04-05 21:16           ` Matt Ellison

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181231221122.1829f0f1@shemminger-XPS-13-9360 \
    --to=stephen@networkplumber.org \
    --cc=matt@arroyo.io \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox