netdev - Re: [PATCH net-next 0/7] openvswitch: add drop reasons

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date: Wed, 26 Jul 2023 10:34:38 +0200
From: Adrian Moreno <amorenoz@...hat.com>
To: Aaron Conole <aconole@...hat.com>
Cc: netdev@...r.kernel.org, dev@...nvswitch.org, i.maximets@....org,
 eric@...ver.life, "David S. Miller" <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>
Subject: Re: [PATCH net-next 0/7] openvswitch: add drop reasons



On 7/24/23 16:34, Aaron Conole wrote:
> Adrian Moreno <amorenoz@...hat.com> writes:
> 
>> There is currently a gap in drop visibility in the openvswitch module.
>> This series tries to improve this by adding a new drop reason subsystem
>> for OVS.
>>
>> Apart from adding a new drop reasson subsystem and some common drop
>> reasons, this series takes Eric's preliminary work [1] on adding an
>> explicit drop action and integrates it into the same subsystem.
>>
>> This series also adds some selftests and so it requires [2] to be
>> applied first.
>>
>> A limitation of this series is that it does not report upcall errors.
>> The reason is that there could be many sources of upcall drops and the
>> most common one, which is the netlink buffer overflow, cannot be
>> reported via kfree_skb() because the skb is freed in the netlink layer
>> (see [3]). Therefore, using a reason for the rare events and not the
>> common one would be even more misleading. I'd propose we add (in a
>> follow up patch) a tracepoint to better report upcall errors.
> 
> I guess you meant to add RFC tag to this, since it depends on other
> series that aren't accepted yet.
> 

Yep, sorry I should have added RFC tag.

> If it's okay, I will pull in your patch 5/7 when I re-post my flow
> additions series, since it will need to be added there at some point
> anyway.
> 

Sure, please go ahead.

>> [1] https://lore.kernel.org/netdev/202306300609.tdRdZscy-lkp@intel.com/T/
>> [2] https://lore.kernel.org/all/9375ccbc-dd40-9998-dde5-c94e4e28f4f1@redhat.com/T/
>> [3] commit 1100248a5c5c ("openvswitch: Fix double reporting of drops in dropwatch")
>>
>> Adrian Moreno (6):
>>    net: openvswitch: add datapath flow drop reason
>>    net: openvswitch: add meter drop reason
>>    net: openvswitch: add misc error drop reasons
>>    selftests: openvswitch: support key masks
>>    selftests: openvswitch: add drop reason testcase
>>    selftests: openvswitch: add explicit drop testcase
>>
>> Eric Garver (1):
>>    net: openvswitch: add explicit drop action
>>
>>   include/net/dropreason.h                      |   6 +
>>   include/uapi/linux/openvswitch.h              |   2 +
>>   net/openvswitch/actions.c                     |  40 ++++--
>>   net/openvswitch/conntrack.c                   |   3 +-
>>   net/openvswitch/datapath.c                    |  16 +++
>>   net/openvswitch/drop.h                        |  33 +++++
>>   net/openvswitch/flow_netlink.c                |   8 +-
>>   .../selftests/net/openvswitch/openvswitch.sh  |  92 +++++++++++++-
>>   .../selftests/net/openvswitch/ovs-dpctl.py    | 115 ++++++++++++------
>>   9 files changed, 267 insertions(+), 48 deletions(-)
>>   create mode 100644 net/openvswitch/drop.h
> 

-- 
Adrián Moreno