[net-next,v2,0/7] net: ip: add drop reasons to input route

Message

Menglong Dong Oct. 7, 2024, 7:46 a.m. UTC

In this series, we mainly add some skb drop reasons to the input path of
ip routing.

The errno from fib_validate_source() is -EINVAL or -EXDEV, and -EXDEV is
used in ip_rcv_finish_core() to increase the LINUX_MIB_IPRPFILTER. For
this case, we can check it by
"drop_reason == SKB_DROP_REASON_IP_RPFILTER" instead. Therefore, we can
make fib_validate_source() return -reason.

Meanwhile, we make the following functions return drop reasons too:

  ip_route_input_mc()
  ip_mc_validate_source()
  ip_route_input_slow()
  ip_route_input_rcu()
  ip_route_input_noref()
  ip_route_input()

And following new skb drop reasons are added:

  SKB_DROP_REASON_IP_LOCAL_SOURCE
  SKB_DROP_REASON_IP_INVALID_SOURCE
  SKB_DROP_REASON_IP_LOCALNET
  SKB_DROP_REASON_IP_INVALID_DEST

Changes since v1:
- make ip_route_input_noref/ip_route_input_rcu/ip_route_input_slow return
  drop reasons, instead of passing a local variable to their function
  arguments.

Menglong Dong (7):
  net: ip: make fib_validate_source() return drop reason
  net: ip: make ip_route_input_mc() return drop reason
  net: ip: make ip_mc_validate_source() return drop reason
  net: ip: make ip_route_input_slow() return drop reasons
  net: ip: make ip_route_input_rcu() return drop reasons
  net: ip: make ip_route_input_noref() return drop reasons
  net: ip: make ip_route_input() return drop reasons

 include/net/dropreason-core.h   |  19 +++++
 include/net/route.h             |  27 ++++---
 net/bridge/br_netfilter_hooks.c |  11 +--
 net/core/lwt_bpf.c              |   1 +
 net/ipv4/fib_frontend.c         |  19 +++--
 net/ipv4/icmp.c                 |   1 +
 net/ipv4/ip_fragment.c          |  12 +--
 net/ipv4/ip_input.c             |  11 ++-
 net/ipv4/route.c                | 131 +++++++++++++++++++-------------
 9 files changed, 145 insertions(+), 87 deletions(-)

Comments

Paolo Abeni Oct. 10, 2024, 8:30 a.m. UTC | #1

On 10/7/24 09:46, Menglong Dong wrote:
> In this series, we mainly add some skb drop reasons to the input path of
> ip routing.
> 
> The errno from fib_validate_source() is -EINVAL or -EXDEV, and -EXDEV is
> used in ip_rcv_finish_core() to increase the LINUX_MIB_IPRPFILTER. For
> this case, we can check it by
> "drop_reason == SKB_DROP_REASON_IP_RPFILTER" instead. Therefore, we can
> make fib_validate_source() return -reason.
> 
> Meanwhile, we make the following functions return drop reasons too:
> 
>    ip_route_input_mc()
>    ip_mc_validate_source()
>    ip_route_input_slow()
>    ip_route_input_rcu()
>    ip_route_input_noref()
>    ip_route_input()

A few other functions are excluded, so that the ip input path coverage 
is not completed - i.e. ip_route_use_hint(), is that intentional?

In any case does not apply cleanly anymore.

Please answer to the above question and question on patch 1 before 
submitting a new revision. At very least the new revision should include 
a comment explaining the reasoning for the current choice.

Please, include in each patch the detailed changelog after the '---' 
separator.

Thanks,

Paolo

Menglong Dong Oct. 10, 2024, 10:32 a.m. UTC | #2

On Thu, Oct 10, 2024 at 4:30 PM Paolo Abeni <[e-mail]> wrote:
>
> On 10/7/24 09:46, Menglong Dong wrote:
> > In this series, we mainly add some skb drop reasons to the input path of
> > ip routing.
> >
> > The errno from fib_validate_source() is -EINVAL or -EXDEV, and -EXDEV is
> > used in ip_rcv_finish_core() to increase the LINUX_MIB_IPRPFILTER. For
> > this case, we can check it by
> > "drop_reason == SKB_DROP_REASON_IP_RPFILTER" instead. Therefore, we can
> > make fib_validate_source() return -reason.
> >
> > Meanwhile, we make the following functions return drop reasons too:
> >
> >    ip_route_input_mc()
> >    ip_mc_validate_source()
> >    ip_route_input_slow()
> >    ip_route_input_rcu()
> >    ip_route_input_noref()
> >    ip_route_input()
>
> A few other functions are excluded, so that the ip input path coverage
> is not completed - i.e. ip_route_use_hint(), is that intentional?
>

Hello,

That's not intentional, I just missed them. At the beginning, I
wanted to organize the drop reasons in ip_route_input_noref(),
and things become complex when I do it. Let me have a check
and make the coverage complete.

> In any case does not apply cleanly anymore.
>
> Please answer to the above question and question on patch 1 before
> submitting a new revision. At very least the new revision should include
> a comment explaining the reasoning for the current choice.
>
> Please, include in each patch the detailed changelog after the '---'
> separator.
>

Sorry about that. I thought the patches for ip_route_input_noref,
ip_route_input_rcu, ip_route_input_slow are completely new one,
and abandoned the changelogs in the patches. I'll complete the
changelogs in the next version.

Thanks!
Menglong Dong


> Thanks,
>
> Paolo
>