All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Ahern <dsahern@gmail.com>
To: Ishaan Gandhi <ishaangandhi@gmail.com>
Cc: David Miller <davem@davemloft.net>,
	Network Development <netdev@vger.kernel.org>,
	willemb@google.com,
	Willem de Bruijn <willemdebruijn.kernel@gmail.com>,
	Stephen Hemminger <stephen@networkplumber.org>
Subject: Re: [PATCH v3] icmp: support rfc5837
Date: Fri, 19 Mar 2021 17:54:54 -0600	[thread overview]
Message-ID: <45eff141-30fb-e8af-5ca5-034a86398ac9@gmail.com> (raw)
In-Reply-To: <5E97397E-7028-46E8-BC0D-44A3E30C41A4@gmail.com>

On 3/19/21 10:11 AM, Ishaan Gandhi wrote:
> Thank you. Would it be better to do instead:
> 
> +	if_index = skb->skb_iif;
> 
> or
> 
> +	if_index = ip_version == 4 ? inet_iif(skb) : skb->skb_iif;
> 

If the packet comes in via an interface assigned to a VRF, skb_iif is
most likely the VRF index which is not what you want.

The general problem of relying on skb_iif was discussed on v1 and v2 of
your patch. Returning an iif that is a VRF, as an example, leaks
information about the networking configuration of the device which from
a quick reading of the RFC is not the intention.

Further, the Security Considerations section recommends controls on what
information can be returned where you have added a single sysctl that
determines if all information or none is returned. Further, it is not a
a per-device control but a global one that applies to all net devices -
though multiple entries per netdevice has a noticeable cost in memory at
scale.

In the end it seems to me the cost benefit is not there for a feature
like this.

  reply	other threads:[~2021-03-19 23:56 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-17 22:19 [PATCH v3] icmp: support rfc5837 ishaangandhi
2021-03-19 14:55 ` David Ahern
2021-03-19 16:11   ` Ishaan Gandhi
2021-03-19 23:54     ` David Ahern [this message]
2021-03-20  0:53       ` Willem de Bruijn
2021-03-20  4:24         ` David Ahern
2021-03-20 20:35           ` rfc5837 and rfc8335 David Ahern
2021-03-22  1:50             ` Ishaan Gandhi
     [not found]               ` <CAJByZJBNMqVDXjcOGCJHGcAv+sT4oEv1FD608TpA_e-J2a3L2w@mail.gmail.com>
     [not found]                 ` <BL0PR05MB5316A2F5C2F1A727FA0190F3AE649@BL0PR05MB5316.namprd05.prod.outlook.com>
2021-03-25  3:19                   ` David Ahern
2021-03-29 14:49                     ` Ron Bonica
2021-03-29 19:39                       ` Ron Bonica
2021-03-31 14:04                         ` Willem de Bruijn
2021-03-31 17:56                           ` Ron Bonica
2021-04-08 22:03                           ` Ishaan Gandhi
2021-05-03  1:41                             ` RESEND " Ishaan Gandhi

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=45eff141-30fb-e8af-5ca5-034a86398ac9@gmail.com \
    --to=dsahern@gmail.com \
    --cc=davem@davemloft.net \
    --cc=ishaangandhi@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=stephen@networkplumber.org \
    --cc=willemb@google.com \
    --cc=willemdebruijn.kernel@gmail.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.