* Re: [RFC net-next] net: dsa: add support for MC_DISABLED attribute
From: Ido Schimmel @ 2019-06-29 15:31 UTC (permalink / raw)
To: f.fainelli, vivien.didelot
Cc: netdev@vger.kernel.org, Jiri Pirko, linux@armlinux.org.uk,
andrew@lunn.ch, davem@davemloft.net
In-Reply-To: <20190623064838.GA13466@splinter>
On Sun, Jun 23, 2019 at 06:48:41AM +0000, Ido Schimmel wrote:
> On Thu, Jun 20, 2019 at 07:24:47PM -0700, Florian Fainelli wrote:
> > On 6/20/2019 4:56 PM, Vivien Didelot wrote:
> > > This patch adds support for enabling or disabling the flooding of
> > > unknown multicast traffic on the CPU ports, depending on the value
> > > of the switchdev SWITCHDEV_ATTR_ID_BRIDGE_MC_DISABLED attribute.
> > >
> > > This allows the user to prevent the CPU to be flooded with a lot of
> > > undesirable traffic that the network stack needs to filter in software.
> > >
> > > The bridge has multicast snooping enabled by default, hence CPU ports
> > > aren't bottlenecked with arbitrary network applications anymore.
> > > But this can be an issue in some scenarios such as pinging the bridge's
> > > IPv6 address. Setting /sys/class/net/br0/bridge/multicast_snooping to
> > > 0 would restore unknown multicast flooding and thus fix ICMPv6. As
> > > an alternative, enabling multicast_querier would program the bridge
> > > address into the switch.
> > From what I can read from mlxsw, we should probably also implement the
> > SWITCHDEV_ATTR_ID_PORT_MROUTER attribute in order to be consistent.
> >
> > Since the attribute MC_DISABLED is on the bridge master, we should also
> > iterate over the list of switch ports being a member of that bridge and
> > change their flooding attribute, taking into account whether
> > BR_MCAST_FLOOD is set on that particular port or not. Just paraphrasing
> > what mlxsw does here again...
>
> When multicast snooping is enabled, unregistered multicast traffic
> should be flooded to mrouter ports only. Otherwise, it should be flooded
> to all ports.
>
> > Once you act on the user-facing ports, you might be able to leave the
> > CPU port flooding unconditionally, since it would only "flood" the CPU
> > port either because an user-facing port has BR_MCAST_FLOOD set, or
> > because this is known MC traffic that got programmed via the bridge's
> > MDB. Would that work?
> >
> > On a higher level, I really wish we did not have to re-implement a lot
> > of identical or similar logic in each switch drivers and had a more
> > central model of what is behaviorally expected.
>
> Well, that model is the bridge driver... But I agree that we can
> probably simplify the interface towards drivers and move more code up
> the stack.
>
> For example, two things mlxsw is doing when multicast snooping is
> enabled:
>
> 1. Writing MDB entries to the device. When multicast snooping is
> disabled, MDB entries are ignored by the bridge driver. Can we agree to
> have the bridge driver generate SWITCHDEV_OBJ_ID_PORT_MDB add / delete
> for all MDB entries when multicast snooping is toggled?
>
> 2. Flooding unregistered multicast traffic only to mrouter ports. The
> bridge driver can iterate over the bridge members and toggle
> BR_MCAST_FLOOD accordingly. It will not actually change this value. Only
> emulate this change towards drivers.
>
> I will try to come up with a more detailed list later this week.
I reviewed the MC logic in mlxsw again and while I found some things
that can be improved, I don't think major simplification can happen
there.
Regarding "central model of what is behaviorally expected". IMO, the
best thing is to make sure that all the implementations pass tests that
codify what is to be expected. Given that the model is the Linux bridge,
the tests should of course pass with veth pairs.
We have some tests under tools/testing/selftests/net/forwarding/, but
not so much for MC. However, even if such tests were to be contributed,
would you be able to run them on your hardware? I remember that in the
past you complained about the number of required ports. Is there
something you can do about it? How many ports are available on your
platforms?
^ permalink raw reply
* BUG: using smp_processor_id() in preemptible [ADDR] code: syz-executor
From: syzbot @ 2019-06-29 15:47 UTC (permalink / raw)
To: allison, davem, gregkh, jon.maloy, kuznet, linux-kernel, netdev,
syzkaller-bugs, tglx, tipc-discussion, ying.xue, yoshfuji
Hello,
syzbot found the following crash on:
HEAD commit: ee7dd773 sis900: remove TxIDLE
git tree: net-next
console output: https://syzkaller.appspot.com/x/log.txt?x=17ceb9a9a00000
kernel config: https://syzkaller.appspot.com/x/.config?x=7ac9edef4d37e5fb
dashboard link: https://syzkaller.appspot.com/bug?extid=1a68504d96cd17b33a05
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=119b2a13a00000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=13127bada00000
The bug was bisected to:
commit 52dfae5c85a4c1078e9f1d5e8947d4a25f73dd81
Author: Jon Maloy <jon.maloy@ericsson.com>
Date: Thu Mar 22 19:42:52 2018 +0000
tipc: obtain node identity from interface by default
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=160ad903a00000
final crash: https://syzkaller.appspot.com/x/report.txt?x=150ad903a00000
console output: https://syzkaller.appspot.com/x/log.txt?x=110ad903a00000
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+1a68504d96cd17b33a05@syzkaller.appspotmail.com
Fixes: 52dfae5c85a4 ("tipc: obtain node identity from interface by default")
Started in network mode
Own node identity 7f000001, cluster identity 4711
New replicast peer: 172.20.20.22
check_preemption_disabled: 3 callbacks suppressed
BUG: using smp_processor_id() in preemptible [00000000] code:
syz-executor834/8612
caller is dst_cache_get+0x3d/0xb0 net/core/dst_cache.c:68
CPU: 0 PID: 8612 Comm: syz-executor834 Not tainted 5.2.0-rc6+ #48
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x172/0x1f0 lib/dump_stack.c:113
check_preemption_disabled lib/smp_processor_id.c:47 [inline]
debug_smp_processor_id+0x251/0x280 lib/smp_processor_id.c:57
dst_cache_get+0x3d/0xb0 net/core/dst_cache.c:68
tipc_udp_xmit.isra.0+0xc4/0xb80 net/tipc/udp_media.c:164
tipc_udp_send_msg+0x29a/0x4b0 net/tipc/udp_media.c:254
tipc_bearer_xmit_skb+0x16c/0x360 net/tipc/bearer.c:503
tipc_enable_bearer+0xabe/0xd20 net/tipc/bearer.c:328
__tipc_nl_bearer_enable+0x2de/0x3a0 net/tipc/bearer.c:899
tipc_nl_bearer_enable+0x23/0x40 net/tipc/bearer.c:907
genl_family_rcv_msg+0x74b/0xf90 net/netlink/genetlink.c:629
genl_rcv_msg+0xca/0x16c net/netlink/genetlink.c:654
netlink_rcv_skb+0x177/0x450 net/netlink/af_netlink.c:2477
genl_rcv+0x29/0x40 net/netlink/genetlink.c:665
netlink_unicast_kernel net/netlink/af_netlink.c:1302 [inline]
netlink_unicast+0x531/0x710 net/netlink/af_netlink.c:1328
netlink_sendmsg+0x8ae/0xd70 net/netlink/af_netlink.c:1917
sock_sendmsg_nosec net/socket.c:646 [inline]
sock_sendmsg+0xd7/0x130 net/socket.c:665
___sys_sendmsg+0x803/0x920 net/socket.c:2286
__sys_sendmsg+0x105/0x1d0 net/socket.c:2324
__do_sys_sendmsg net/socket.c:2333 [inline]
__se_sys_sendmsg net/socket.c:2331 [inline]
__x64_sys_sendmsg+0x78/0xb0 net/socket.c:2331
do_syscall_64+0xfd/0x680 arch/x86/entry/common.c:301
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x444679
Code: 18 89 d0 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 00 48 89 f8 48 89 f7
48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff
ff 0f 83 1b d8 fb ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007fff0201a8b8 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 00000000004002e0 RCX: 0000000000444679
RDX: 0000000000000000 RSI: 0000000020000580 RDI: 0000000000000003
RBP: 00000000006cf018 R08: 0000000000000001 R09: 00000000004002e0
R10: 0000000000000008 R11: 0000000000000246 R12: 0000000000402320
R13: 00000000004023b0 R14: 0000000000000000 R15: 0000000000
---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches
^ permalink raw reply
* Re: [RFC net-next] net: dsa: add support for MC_DISABLED attribute
From: Ido Schimmel @ 2019-06-29 16:29 UTC (permalink / raw)
To: Russell King - ARM Linux admin, nikolay, linus.luessing
Cc: Ido Schimmel, Vivien Didelot, Florian Fainelli,
netdev@vger.kernel.org, Jiri Pirko, andrew@lunn.ch,
davem@davemloft.net
In-Reply-To: <20190623074427.GA21875@splinter>
On Sun, Jun 23, 2019 at 10:44:27AM +0300, Ido Schimmel wrote:
> On Sun, Jun 23, 2019 at 08:26:05AM +0100, Russell King - ARM Linux admin wrote:
> > On Sun, Jun 23, 2019 at 07:09:52AM +0000, Ido Schimmel wrote:
> > > When multicast snooping is enabled unregistered multicast traffic should
> > > only be flooded to mrouter ports.
> >
> > Given that IPv6 relies upon multicast working, and multicast snooping
> > is a kernel configuration option, and MLD messages will only be sent
> > when whenever the configuration on the target changes, and there may
> > not be a multicast querier in the system, who does that ensure that
> > IPv6 can work on a bridge where the kernel configured and built with
> > multicast snooping enabled?
>
> See commit b00589af3b04 ("bridge: disable snooping if there is no
> querier"). I think that's unfortunate behavior that we need because
> multicast snooping is enabled by default. If it weren't enabled by
> default, then anyone enabling it would also make sure there's a querier
> in the network.
Linus, Nik,
I brought this problem in the past, but we didn't reach a solution, so
I'll try again :)
The problem:
Even if multicast snooping is enabled, the bridge driver will flood
multicast packets to all the ports if no querier was detected on the
link. The querier states (IPv4 & IPv6) are not currently reflected to
switchdev drivers which means that the hardware data path will only
flood unregistered multicast packets to mrouter ports (which can be an
empty list).
In default configurations (where multicast snooping is enabled and the
bridge querier is disabled), this can prevent IPv6 ping from passing, as
there are no mrouter ports and there is no MDB entry corresponding to
the solicited-node multicast address.
Is there anything we can do about it? Enable the bridge querier if no
other querier was detected? Commit c5c23260594c ("bridge: Add
multicast_querier toggle and disable queries by default") disabled
queries by default, but I'm only suggesting to turn them on if no other
querier was detected on the link. Do you think it's still a problem?
I would like to avoid having drivers take the querier state into account
as it will only complicate things further.
Thanks
^ permalink raw reply
* Re: [PATCH V2] include: linux: Regularise the use of FIELD_SIZEOF macro
From: Joe Perches @ 2019-06-29 16:45 UTC (permalink / raw)
To: Alexey Dobriyan, Andreas Dilger
Cc: Andrew Morton, Shyam Saini, kernel-hardening, linux-kernel,
keescook, linux-arm-kernel, linux-mips, intel-gvt-dev, intel-gfx,
dri-devel, netdev, linux-ext4, devel, linux-mm, linux-sctp, bpf,
kvm, mayhs11saini
In-Reply-To: <20190629142510.GA10629@avx2>
On Sat, 2019-06-29 at 17:25 +0300, Alexey Dobriyan wrote:
> On Tue, Jun 11, 2019 at 03:00:10PM -0600, Andreas Dilger wrote:
> > On Jun 11, 2019, at 2:48 PM, Andrew Morton <akpm@linux-foundation.org> wrote:
> > > On Wed, 12 Jun 2019 01:08:36 +0530 Shyam Saini <shyam.saini@amarulasolutions.com> wrote:
> > I did a check, and FIELD_SIZEOF() is used about 350x, while sizeof_field()
> > is about 30x, and SIZEOF_FIELD() is only about 5x.
> >
> > That said, I'm much more in favour of "sizeof_field()" or "sizeof_member()"
> > than FIELD_SIZEOF(). Not only does that better match "offsetof()", with
> > which it is closely related, but is also closer to the original "sizeof()".
> >
> > Since this is a rather trivial change, it can be split into a number of
> > patches to get approval/landing via subsystem maintainers, and there is no
> > huge urgency to remove the original macros until the users are gone. It
> > would make sense to remove SIZEOF_FIELD() and sizeof_field() quickly so
> > they don't gain more users, and the remaining FIELD_SIZEOF() users can be
> > whittled away as the patches come through the maintainer trees.
>
> The signature should be
>
> sizeof_member(T, m)
>
> it is proper English,
> it is lowercase, so is easier to type,
> it uses standard term (member, not field),
> it blends in with standard "sizeof" operator,
yes please.
Also, a simple script conversion applied
immediately after an rc1 might be easiest
rather than individual patches.
^ permalink raw reply
* Re: [PATCH net-next v2 2/4] gve: Add transmit and receive support
From: kbuild test robot @ 2019-06-29 17:00 UTC (permalink / raw)
To: Catherine Sullivan
Cc: kbuild-all, netdev, Catherine Sullivan, Sagi Shahar, Jon Olson,
Willem de Bruijn, Luigi Rizzo
In-Reply-To: <20190628175633.143501-3-csully@google.com>
[-- Attachment #1: Type: text/plain, Size: 1353 bytes --]
Hi Catherine,
I love your patch! Yet something to improve:
[auto build test ERROR on net-next/master]
url: https://github.com/0day-ci/linux/commits/Catherine-Sullivan/Add-gve-driver/20190629-143743
config: i386-allyesconfig (attached as .config)
compiler: gcc-7 (Debian 7.4.0-9) 7.4.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
In file included from drivers/net/ethernet/google/gve/gve_main.c:16:0:
>> drivers/net/ethernet/google/gve/gve_adminq.h:134:1: error: static assertion failed: "sizeof(struct gve_adminq_create_rx_queue) == 48"
static_assert(sizeof(struct gve_adminq_create_rx_queue) == 48);
^~~~~~~~~~~~~~
drivers/net/ethernet/google/gve/gve_adminq.h:171:1: error: static assertion failed: "sizeof(struct gve_adminq_set_driver_parameter) == 16"
static_assert(sizeof(struct gve_adminq_set_driver_parameter) == 16);
^~~~~~~~~~~~~~
vim +134 drivers/net/ethernet/google/gve/gve_adminq.h
133
> 134 static_assert(sizeof(struct gve_adminq_create_rx_queue) == 48);
135
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 67813 bytes --]
^ permalink raw reply
* Re: [PATCH net-next v3 4/4] gve: Add ethtool support
From: David Miller @ 2019-06-29 17:19 UTC (permalink / raw)
To: csully; +Cc: netdev, sagis, jonolson, willemb, lrizzo
In-Reply-To: <20190628230733.54169-5-csully@google.com>
From: Catherine Sullivan <csully@google.com>
Date: Fri, 28 Jun 2019 16:07:33 -0700
> +void gve_get_channels(struct net_device *netdev, struct ethtool_channels *cmd)
...
> +int gve_set_channels(struct net_device *netdev, struct ethtool_channels *cmd)
...
> +void gve_get_ringparam(struct net_device *netdev,
> + struct ethtool_ringparam *cmd)
...
> +int gve_user_reset(struct net_device *netdev, u32 *flags)
...
Please mark these static as per the kbuild robot feedback.
Thank you.
^ permalink raw reply
* Re: [PATCH net-next] hinic: add vlan offload support
From: David Miller @ 2019-06-29 17:28 UTC (permalink / raw)
To: xuechaojing
Cc: linux-kernel, netdev, luoshaokai, cloud.wangxiaoyun, chiqijun,
wulike1
In-Reply-To: <20190629022627.25396-1-xuechaojing@huawei.com>
From: Xue Chaojing <xuechaojing@huawei.com>
Date: Sat, 29 Jun 2019 02:26:27 +0000
> This patch adds vlan offload support for the HINIC driver.
>
> Signed-off-by: Xue Chaojing <xuechaojing@huawei.com>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH net-next 0/2] Fix batched event generation for mirred action
From: Jamal Hadi Salim @ 2019-06-29 17:45 UTC (permalink / raw)
To: Roman Mashak, davem; +Cc: netdev, kernel, xiyou.wangcong, jiri
In-Reply-To: <1561746618-29349-1-git-send-email-mrv@mojatatu.com>
On 2019-06-28 2:30 p.m., Roman Mashak wrote:
> When adding or deleting a batch of entries, the kernel sends upto
> TCA_ACT_MAX_PRIO entries in an event to user space. However it does not
> consider that the action sizes may vary and require different skb sizes.
>
> For example :
>
> % cat tc-batch.sh
> TC="sudo /mnt/iproute2.git/tc/tc"
>
> $TC actions flush action mirred
> for i in `seq 1 $1`;
> do
> cmd="action mirred egress redirect dev lo index $i "
> args=$args$cmd
> done
> $TC actions add $args
> %
> % ./tc-batch.sh 32
> Error: Failed to fill netlink attributes while adding TC action.
> We have an error talking to the kernel
> %
>
> patch 1 adds callback in tc_action_ops of mirred action, which calculates
> the action size, and passes size to tcf_add_notify()/tcf_del_notify().
>
> patch 2 updates the TDC test suite with relevant test cases.
>
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
cheers,
jamal
^ permalink raw reply
* Re: [PATCH net] sctp: not bind the socket in sctp_connect
From: David Miller @ 2019-06-29 17:51 UTC (permalink / raw)
To: lucien.xin; +Cc: netdev, linux-sctp, marcelo.leitner, nhorman, syzkaller-bugs
In-Reply-To: <35a0e4f6ca68185117c6e5517d8ac924cc2f9d05.1561537899.git.lucien.xin@gmail.com>
From: Xin Long <lucien.xin@gmail.com>
Date: Wed, 26 Jun 2019 16:31:39 +0800
> Now when sctp_connect() is called with a wrong sa_family, it binds
> to a port but doesn't set bp->port, then sctp_get_af_specific will
> return NULL and sctp_connect() returns -EINVAL.
>
> Then if sctp_bind() is called to bind to another port, the last
> port it has bound will leak due to bp->port is NULL by then.
>
> sctp_connect() doesn't need to bind ports, as later __sctp_connect
> will do it if bp->port is NULL. So remove it from sctp_connect().
> While at it, remove the unnecessary sockaddr.sa_family len check
> as it's already done in sctp_inet_connect.
>
> Fixes: 644fbdeacf1d ("sctp: fix the issue that flags are ignored when using kernel_connect")
> Reported-by: syzbot+079bf326b38072f849d9@syzkaller.appspotmail.com
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH net-next] xdp: xdp_umem: fix umem pages mapping for 32bits systems
From: David Miller @ 2019-06-29 17:53 UTC (permalink / raw)
To: bjorn.topel
Cc: ivan.khoronzhuk, bjorn.topel, magnus.karlsson, ast, daniel, hawk,
john.fastabend, netdev, bpf, xdp-newbies, linux-kernel
In-Reply-To: <CAJ+HfNid3PntipAJHuPR-tQudf+E6UQK6mPDHdc0O=wCUSjEEA@mail.gmail.com>
From: Björn Töpel <bjorn.topel@gmail.com>
Date: Wed, 26 Jun 2019 22:50:23 +0200
> On Wed, 26 Jun 2019 at 17:59, Ivan Khoronzhuk
> <ivan.khoronzhuk@linaro.org> wrote:
>>
>> Use kmap instead of page_address as it's not always in low memory.
>>
>
> Ah, some 32-bit love. :-) Thanks for working on this!
>
> For future patches, please base AF_XDP patches on the bpf/bpf-next
> tree instead of net/net-next.
>
> Acked-by: Björn Töpel <bjorn.topel@intel.com>
Alexei and Daniel, I'll let you guys take this one.
Thanks.
^ permalink raw reply
* [PATCH] ipv4: Fix off-by-one in route dump counter without netlink strict checking
From: Stefano Brivio @ 2019-06-29 17:55 UTC (permalink / raw)
To: David Miller; +Cc: David Ahern, netdev
In commit ee28906fd7a1 ("ipv4: Dump route exceptions if requested") I
added a counter of per-node dumped routes (including actual routes and
exceptions), analogous to the existing counter for dumped nodes. Dumping
exceptions means we need to also keep track of how many routes are dumped
for each node: this would be just one route per node, without exceptions.
When netlink strict checking is not enabled, we dump both routes and
exceptions at the same time: the RTM_F_CLONED flag is not used as a
filter. In this case, the per-node counter 'i_fa' is incremented by one
to track the single dumped route, then also incremented by one for each
exception dumped, and then stored as netlink callback argument as skip
counter, 's_fa', to be used when a partial dump operation restarts.
The per-node counter needs to be increased by one also when we skip a
route (exception) due to a previous non-zero skip counter, because it
needs to match the existing skip counter, if we are dumping both routes
and exceptions. I missed this, and only incremented the counter, for
regular routes, if the previous skip counter was zero. This means that,
in case of a mixed dump, partial dump operations after the first one
will start with a mismatching skip counter value, one less than expected.
This means in turn that the first exception for a given node is skipped
every time a partial dump operation restarts, if netlink strict checking
is not enabled (iproute < 5.0).
It turns out I didn't repeat the test in its final version, commit
de755a85130e ("selftests: pmtu: Introduce list_flush_ipv4_exception test
case"), which also counts the number of route exceptions returned, with
iproute2 versions < 5.0 -- I was instead using the equivalent of the IPv6
test as it was before commit b964641e9925 ("selftests: pmtu: Make
list_flush_ipv6_exception test more demanding").
Always increment the per-node counter by one if we previously dumped
a regular route, so that it matches the current skip counter.
Fixes: ee28906fd7a1 ("ipv4: Dump route exceptions if requested")
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
net/ipv4/fib_trie.c | 22 ++++++++++++++--------
1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
index 4400f5051977..2b2b3d291ab0 100644
--- a/net/ipv4/fib_trie.c
+++ b/net/ipv4/fib_trie.c
@@ -2126,14 +2126,20 @@ static int fn_trie_dump_leaf(struct key_vector *l, struct fib_table *tb,
goto next;
}
- if (filter->dump_routes && !s_fa) {
- err = fib_dump_info(skb, NETLINK_CB(cb->skb).portid,
- cb->nlh->nlmsg_seq, RTM_NEWROUTE,
- tb->tb_id, fa->fa_type,
- xkey, KEYLENGTH - fa->fa_slen,
- fa->fa_tos, fi, flags);
- if (err < 0)
- goto stop;
+ if (filter->dump_routes) {
+ if (!s_fa) {
+ err = fib_dump_info(skb,
+ NETLINK_CB(cb->skb).portid,
+ cb->nlh->nlmsg_seq,
+ RTM_NEWROUTE,
+ tb->tb_id, fa->fa_type,
+ xkey,
+ KEYLENGTH - fa->fa_slen,
+ fa->fa_tos, fi, flags);
+ if (err < 0)
+ goto stop;
+ }
+
i_fa++;
}
--
2.20.1
^ permalink raw reply related
* Re: [PATCH net v2] net: make skb_dst_force return true when dst is refcounted
From: David Miller @ 2019-06-29 18:02 UTC (permalink / raw)
To: fw; +Cc: netdev, eric.dumazet, netfilter-devel
In-Reply-To: <20190626184045.2922-1-fw@strlen.de>
From: Florian Westphal <fw@strlen.de>
Date: Wed, 26 Jun 2019 20:40:45 +0200
> netfilter did not expect that skb_dst_force() can cause skb to lose its
> dst entry.
>
> I got a bug report with a skb->dst NULL dereference in netfilter
> output path. The backtrace contains nf_reinject(), so the dst might have
> been cleared when skb got queued to userspace.
>
> Other users were fixed via
> if (skb_dst(skb)) {
> skb_dst_force(skb);
> if (!skb_dst(skb))
> goto handle_err;
> }
>
> But I think its preferable to make the 'dst might be cleared' part
> of the function explicit.
>
> In netfilter case, skb with a null dst is expected when queueing in
> prerouting hook, so drop skb for the other hooks.
>
> v2:
> v1 of this patch returned true in case skb had no dst entry.
> Eric said:
> Say if we have two skb_dst_force() calls for some reason
> on the same skb, only the first one will return false.
>
> This now returns false even when skb had no dst, as per Erics
> suggestion, so callers might need to check skb_dst() first before
> skb_dst_force().
>
> Signed-off-by: Florian Westphal <fw@strlen.de>
...
> Alternatively this could be routed via netfilter tree, let me
> know your preference.
Applied and I'll queue this up for -stable, thanks Florian.
^ permalink raw reply
* Re: [PATCH bpf] selftests: bpf: fix inlines in test_lwt_seg6local
From: Song Liu @ 2019-06-29 18:04 UTC (permalink / raw)
To: Jiri Benc
Cc: Networking, bpf, Alexei Starovoitov, Daniel Borkmann,
Mathieu Xhonneux
In-Reply-To: <4fdda0547f90e96bd2ef5d5533ee286b02dd4ce2.1561819374.git.jbenc@redhat.com>
On Sat, Jun 29, 2019 at 7:43 AM Jiri Benc <jbenc@redhat.com> wrote:
>
> Selftests are reporting this failure in test_lwt_seg6local.sh:
>
> + ip netns exec ns2 ip -6 route add fb00::6 encap bpf in obj test_lwt_seg6local.o sec encap_srh dev veth2
> Error fetching program/map!
> Failed to parse eBPF program: Operation not permitted
>
> The problem is __attribute__((always_inline)) alone is not enough to prevent
> clang from inserting those functions in .text. In that case, .text is not
> marked as relocateable.
>
> See the output of objdump -h test_lwt_seg6local.o:
>
> Idx Name Size VMA LMA File off Algn
> 0 .text 00003530 0000000000000000 0000000000000000 00000040 2**3
> CONTENTS, ALLOC, LOAD, READONLY, CODE
>
> This causes the iproute bpf loader to fail in bpf_fetch_prog_sec:
> bpf_has_call_data returns true but bpf_fetch_prog_relo fails as there's no
> relocateable .text section in the file.
>
> Add 'static inline' to fix this.
>
> Fixes: c99a84eac026 ("selftests/bpf: test for seg6local End.BPF action")
> Signed-off-by: Jiri Benc <jbenc@redhat.com>
Maybe use "__always_inline" as most other tests do?
Thanks,
Song
^ permalink raw reply
* Re: [PATCH bpf] selftests: bpf: fix inlines in test_lwt_seg6local
From: Song Liu @ 2019-06-29 18:04 UTC (permalink / raw)
To: Jiri Benc
Cc: Networking, bpf, Alexei Starovoitov, Daniel Borkmann,
Mathieu Xhonneux
In-Reply-To: <CAPhsuW4ncpfNCvbYHF36pb6ZEBJMX-iJP5sD0x3PbmAds+WGOQ@mail.gmail.com>
On Sat, Jun 29, 2019 at 11:04 AM Song Liu <liu.song.a23@gmail.com> wrote:
>
> On Sat, Jun 29, 2019 at 7:43 AM Jiri Benc <jbenc@redhat.com> wrote:
> >
> > Selftests are reporting this failure in test_lwt_seg6local.sh:
> >
> > + ip netns exec ns2 ip -6 route add fb00::6 encap bpf in obj test_lwt_seg6local.o sec encap_srh dev veth2
> > Error fetching program/map!
> > Failed to parse eBPF program: Operation not permitted
> >
> > The problem is __attribute__((always_inline)) alone is not enough to prevent
> > clang from inserting those functions in .text. In that case, .text is not
> > marked as relocateable.
> >
> > See the output of objdump -h test_lwt_seg6local.o:
> >
> > Idx Name Size VMA LMA File off Algn
> > 0 .text 00003530 0000000000000000 0000000000000000 00000040 2**3
> > CONTENTS, ALLOC, LOAD, READONLY, CODE
> >
> > This causes the iproute bpf loader to fail in bpf_fetch_prog_sec:
> > bpf_has_call_data returns true but bpf_fetch_prog_relo fails as there's no
> > relocateable .text section in the file.
> >
> > Add 'static inline' to fix this.
> >
> > Fixes: c99a84eac026 ("selftests/bpf: test for seg6local End.BPF action")
> > Signed-off-by: Jiri Benc <jbenc@redhat.com>
>
> Maybe use "__always_inline" as most other tests do?
I meant "static __always_inline".
Song
^ permalink raw reply
* Re: [Linux-kernel-mentees][PATCH v2] packet: Fix undefined behavior in bit shift
From: David Miller @ 2019-06-29 18:06 UTC (permalink / raw)
To: c0d1n61at3; +Cc: skhan, linux-kernel-mentees, netdev, linux-kernel
In-Reply-To: <20190627032532.18374-2-c0d1n61at3@gmail.com>
From: Jiunn Chang <c0d1n61at3@gmail.com>
Date: Wed, 26 Jun 2019 22:25:30 -0500
> Shifting signed 32-bit value by 31 bits is undefined. Changing most
> significant bit to unsigned.
>
> Changes included in v2:
> - use subsystem specific subject lines
> - CC required mailing lists
>
> Signed-off-by: Jiunn Chang <c0d1n61at3@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH 0/2] Sub ns increment fixes in Macb PTP
From: David Miller @ 2019-06-29 18:09 UTC (permalink / raw)
To: harini.katakam
Cc: nicolas.ferre, richardcochran, claudiu.beznea, rafalo,
andrei.pistirica, netdev, linux-kernel, michal.simek,
harinikatakamlinux
In-Reply-To: <1561616460-32439-1-git-send-email-harini.katakam@xilinx.com>
From: Harini Katakam <harini.katakam@xilinx.com>
Date: Thu, 27 Jun 2019 11:50:58 +0530
> The subns increment register fields are not captured correctly in the
> driver. Fix the same and also increase the subns incr resolution.
>
> Sub ns resolution was increased to 24 bits in r1p06f2 version. To my
> knowledge, this PTP driver, with its current BD time stamp
> implementation, is only useful to that version or above. So, I have
> increased the resolution unconditionally. Please let me know if there
> is any IP versions incompatible with this - there is no register to
> obtain this information from.
>
> Changes from RFC:
> None
Series applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next v3 0/4] em_ipt: add support for addrtype
From: David Miller @ 2019-06-29 18:15 UTC (permalink / raw)
To: nikolay; +Cc: netdev, roopa, pablo, xiyou.wangcong, jiri, jhs, eyal.birger
In-Reply-To: <20190627081047.24537-1-nikolay@cumulusnetworks.com>
From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Date: Thu, 27 Jun 2019 11:10:43 +0300
> We would like to be able to use the addrtype from tc for ACL rules and
> em_ipt seems the best place to add support for the already existing xt
> match. The biggest issue is that addrtype revision 1 (with ipv6 support)
> is NFPROTO_UNSPEC and currently em_ipt can't differentiate between v4/v6
> if such xt match is used because it passes the match's family instead of
> the packet one. The first 3 patches make em_ipt match only on IP
> traffic (currently both policy and addrtype recognize such traffic
> only) and make it pass the actual packet's protocol instead of the xt
> match family when it's unspecified. They also add support for NFPROTO_UNSPEC
> xt matches. The last patch allows to add addrtype rules via em_ipt.
> We need to keep the user-specified nfproto for dumping in order to be
> compatible with libxtables, we cannot dump NFPROTO_UNSPEC as the nfproto
> or we'll get an error from libxtables, thus the nfproto is limited to
> ipv4/ipv6 in patch 03 and is recorded.
>
> v3: don't use the user nfproto for matching, only for dumping, more
> information is available in the commit message in patch 03
> v2: change patch 02 to set the nfproto only when unspecified and drop
> patch 04 from v1 (Eyal Birger)
Series applied, thanks Nikolay.
^ permalink raw reply
* Re: [PATCH net] igmp: fix memory leak in igmpv3_del_delrec()
From: David Miller @ 2019-06-29 18:17 UTC (permalink / raw)
To: edumazet; +Cc: netdev, eric.dumazet, liuhangbin, syzbot+6ca1abd0db68b5173a4f
In-Reply-To: <20190627082701.226711-1-edumazet@google.com>
From: Eric Dumazet <edumazet@google.com>
Date: Thu, 27 Jun 2019 01:27:01 -0700
> im->tomb and/or im->sources might not be NULL, but we
> currently overwrite their values blindly.
>
> Using swap() will make sure the following call to kfree_pmc(pmc)
> will properly free the psf structures.
>
> Tested with the C repro provided by syzbot, which basically does :
>
> socket(PF_INET, SOCK_DGRAM, IPPROTO_IP) = 3
> setsockopt(3, SOL_IP, IP_ADD_MEMBERSHIP, "\340\0\0\2\177\0\0\1\0\0\0\0", 12) = 0
> ioctl(3, SIOCSIFFLAGS, {ifr_name="lo", ifr_flags=0}) = 0
> setsockopt(3, SOL_IP, IP_MSFILTER, "\340\0\0\2\177\0\0\1\1\0\0\0\1\0\0\0\377\377\377\377", 20) = 0
> ioctl(3, SIOCSIFFLAGS, {ifr_name="lo", ifr_flags=IFF_UP}) = 0
> exit_group(0) = ?
>
> BUG: memory leak
> unreferenced object 0xffff88811450f140 (size 64):
...
> Fixes: 24803f38a5c0 ("igmp: do not remove igmp souce list info when set link down")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Hangbin Liu <liuhangbin@gmail.com>
> Reported-by: syzbot+6ca1abd0db68b5173a4f@syzkaller.appspotmail.com
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH] ipv4: Fix off-by-one in route dump counter without netlink strict checking
From: Stefano Brivio @ 2019-06-29 18:21 UTC (permalink / raw)
To: David Miller; +Cc: David Ahern, netdev
In-Reply-To: <74faa085e6af026f8b9f0d3cce8a94147781f257.1561830851.git.sbrivio@redhat.com>
On Sat, 29 Jun 2019 19:55:08 +0200
Stefano Brivio <sbrivio@redhat.com> wrote:
> Always increment the per-node counter by one if we previously dumped
> a regular route, so that it matches the current skip counter.
>
> Fixes: ee28906fd7a1 ("ipv4: Dump route exceptions if requested")
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Sorry David, this was meant for net-next.
--
Stefano
^ permalink raw reply
* Re: [PATCH v3 bpf-next 4/4] tools/bpftool: switch map event_pipe to libbpf's perf_buffer
From: Jakub Kicinski @ 2019-06-29 18:24 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: andrii.nakryiko, ast, daniel, bpf, netdev, kernel-team,
songliubraving
In-Reply-To: <20190629055309.1594755-5-andriin@fb.com>
On Fri, 28 Jun 2019 22:53:09 -0700, Andrii Nakryiko wrote:
> map_info_len = sizeof(map_info);
> map_fd = map_parse_fd_and_info(&argc, &argv, &map_info, &map_info_len);
> - if (map_fd < 0)
> + if (map_fd < 0) {
> + p_err("failed to get map info");
Can't do, map_parse_fd_and_info() prints an error already, we can't
have multiple errors in JSON.
> return -1;
> + }
>
> if (map_info.type != BPF_MAP_TYPE_PERF_EVENT_ARRAY) {
> p_err("map is not a perf event array");
> @@ -205,7 +157,7 @@ int do_event_pipe(int argc, char **argv)
> char *endptr;
>
> NEXT_ARG();
> - cpu = strtoul(*argv, &endptr, 0);
> + ctx.cpu = strtoul(*argv, &endptr, 0);
> if (*endptr) {
> p_err("can't parse %s as CPU ID", **argv);
> goto err_close_map;
> @@ -216,7 +168,7 @@ int do_event_pipe(int argc, char **argv)
> char *endptr;
>
> NEXT_ARG();
> - index = strtoul(*argv, &endptr, 0);
> + ctx.idx = strtoul(*argv, &endptr, 0);
> if (*endptr) {
> p_err("can't parse %s as index", **argv);
> goto err_close_map;
> @@ -228,45 +180,32 @@ int do_event_pipe(int argc, char **argv)
> goto err_close_map;
> }
>
> - do_all = false;
> + ctx.all_cpus = false;
> }
>
> - if (!do_all) {
> - if (index == -1 || cpu == -1) {
> + if (!ctx.all_cpus) {
> + if (ctx.idx == -1 || ctx.cpu == -1) {
> p_err("cpu and index must be specified together");
> goto err_close_map;
Now that you look at err looks like we're missing an err = -1 assignment
here? but...
> }
> -
> - nfds = 1;
> } else {
> - nfds = min(get_possible_cpus(), map_info.max_entries);
> - cpu = 0;
> - index = 0;
> + ctx.cpu = 0;
> + ctx.idx = 0;
> }
>
> - rings = calloc(nfds, sizeof(rings[0]));
> - if (!rings)
> + opts.attr = &perf_attr;
> + opts.event_cb = print_bpf_output;
> + opts.ctx = &ctx;
> + opts.cpu_cnt = ctx.all_cpus ? 0 : 1;
> + opts.cpus = &ctx.cpu;
> + opts.map_keys = &ctx.idx;
> +
> + pb = perf_buffer__new_raw(map_fd, MMAP_PAGE_CNT, &opts);
> + err = libbpf_get_error(pb);
> + if (err) {
> + p_err("failed to create perf buffer: %s (%d)",
> + strerror(err), err);
> goto err_close_map;
> -
> - pfds = calloc(nfds, sizeof(pfds[0]));
> - if (!pfds)
> - goto err_free_rings;
> -
> - for (i = 0; i < nfds; i++) {
> - rings[i].cpu = cpu + i;
> - rings[i].key = index + i;
> -
> - rings[i].fd = bpf_perf_event_open(map_fd, rings[i].key,
> - rings[i].cpu);
> - if (rings[i].fd < 0)
> - goto err_close_fds_prev;
> -
> - rings[i].mem = perf_event_mmap(rings[i].fd);
> - if (!rings[i].mem)
> - goto err_close_fds_current;
> -
> - pfds[i].fd = rings[i].fd;
> - pfds[i].events = POLLIN;
> }
>
> signal(SIGINT, int_exit);
> @@ -277,35 +216,25 @@ int do_event_pipe(int argc, char **argv)
> jsonw_start_array(json_wtr);
>
> while (!stop) {
> - poll(pfds, nfds, 200);
> - for (i = 0; i < nfds; i++)
> - perf_event_read(&rings[i], &tmp_buf, &tmp_buf_sz);
> + err = perf_buffer__poll(pb, 200);
> + if (err < 0 && err != -EINTR) {
> + p_err("perf buffer polling failed: %s (%d)",
> + strerror(err), err);
> + goto err_close_pb;
> + }
> }
> - free(tmp_buf);
>
> if (json_output)
> jsonw_end_array(json_wtr);
>
> - for (i = 0; i < nfds; i++) {
> - perf_event_unmap(rings[i].mem);
> - close(rings[i].fd);
> - }
> - free(pfds);
> - free(rings);
> + perf_buffer__free(pb);
> close(map_fd);
>
> return 0;
>
> -err_close_fds_prev:
> - while (i--) {
> - perf_event_unmap(rings[i].mem);
> -err_close_fds_current:
> - close(rings[i].fd);
> - }
> - free(pfds);
> -err_free_rings:
> - free(rings);
> +err_close_pb:
> + perf_buffer__free(pb);
> err_close_map:
> close(map_fd);
> - return -1;
> + return err ? -1 : 0;
... how can we return 0 on the error path? 😕
> }
^ permalink raw reply
* Re: [PATCH net-next v3 0/4] Add gve driver
From: Jakub Kicinski @ 2019-06-29 18:28 UTC (permalink / raw)
To: Catherine Sullivan; +Cc: netdev
In-Reply-To: <20190628230733.54169-1-csully@google.com>
On Fri, 28 Jun 2019 16:07:29 -0700, Catherine Sullivan wrote:
> This patch series adds the gve driver which will support the
> Compute Engine Virtual NIC that will be available in the future.
Looks like buildbot also found a bunch of bad endian casts.
Please make sure the driver builds with no warnings with W=1 C=1.
^ permalink raw reply
* Re: [PATCH v2 net-next 04/19] ionic: Add basic lif support
From: Jakub Kicinski @ 2019-06-29 18:45 UTC (permalink / raw)
To: Shannon Nelson; +Cc: netdev
In-Reply-To: <20190628213934.8810-5-snelson@pensando.io>
On Fri, 28 Jun 2019 14:39:19 -0700, Shannon Nelson wrote:
> @@ -64,4 +66,49 @@ int ionic_debugfs_add_ident(struct ionic *ionic)
> ionic, &identity_fops) ? 0 : -ENOTSUPP;
> }
>
> +int ionic_debugfs_add_sizes(struct ionic *ionic)
> +{
> + debugfs_create_u32("nlifs", 0400, ionic->dentry,
> + (u32 *)&ionic->ident.dev.nlifs);
> + debugfs_create_u32("nintrs", 0400, ionic->dentry, &ionic->nintrs);
> +
> + debugfs_create_u32("ntxqs_per_lif", 0400, ionic->dentry,
> + (u32 *)&ionic->ident.lif.eth.config.queue_count[IONIC_QTYPE_TXQ]);
> + debugfs_create_u32("nrxqs_per_lif", 0400, ionic->dentry,
> + (u32 *)&ionic->ident.lif.eth.config.queue_count[IONIC_QTYPE_RXQ]);
Are these __le32s? Does the driver build cleanly with W=1 C=1?
> + return 0;
> +}
^ permalink raw reply
* Re: [PATCH v2 net-next 17/19] ionic: Add RSS support
From: Jakub Kicinski @ 2019-06-29 18:48 UTC (permalink / raw)
To: Shannon Nelson; +Cc: netdev
In-Reply-To: <20190628213934.8810-18-snelson@pensando.io>
On Fri, 28 Jun 2019 14:39:32 -0700, Shannon Nelson wrote:
> @@ -1260,10 +1266,24 @@ static struct lif *ionic_lif_alloc(struct ionic *ionic, unsigned int index)
> if (err)
> goto err_out_free_lif_info;
>
> + /* allocate rss indirection table */
> + tbl_sz = le16_to_cpu(lif->ionic->ident.lif.eth.rss_ind_tbl_sz);
> + lif->rss_ind_tbl_sz = sizeof(*lif->rss_ind_tbl) * tbl_sz;
> + lif->rss_ind_tbl = dma_alloc_coherent(dev, lif->rss_ind_tbl_sz,
> + &lif->rss_ind_tbl_pa,
> + GFP_KERNEL);
> +
> + if (!lif->rss_ind_tbl) {
> + dev_err(dev, "Failed to allocate rss indirection table, aborting\n");
> + goto err_out_free_qcqs;
> + }
> +
> list_add_tail(&lif->list, &ionic->lifs);
>
> return lif;
>
> +err_out_free_qcqs:
> + ionic_qcqs_free(lif);
> err_out_free_lif_info:
> dma_free_coherent(dev, lif->info_sz, lif->info, lif->info_pa);
> lif->info = NULL;
> @@ -1302,6 +1322,14 @@ static void ionic_lif_free(struct lif *lif)
> {
> struct device *dev = lif->ionic->dev;
>
> + /* free rss indirection table */
> + if (lif->rss_ind_tbl) {
> + dma_free_coherent(dev, lif->rss_ind_tbl_sz, lif->rss_ind_tbl,
> + lif->rss_ind_tbl_pa);
dma_free_coherent() should be able to deal with NULLs just fine.
Besides you fail hard if the allocation failed, no?
> + lif->rss_ind_tbl = NULL;
> + lif->rss_ind_tbl_pa = 0;
> + }
^ permalink raw reply
* Re: [PATCH v2 net-next 16/19] ionic: Add driver stats
From: Jakub Kicinski @ 2019-06-29 18:53 UTC (permalink / raw)
To: Shannon Nelson; +Cc: netdev
In-Reply-To: <20190628213934.8810-17-snelson@pensando.io>
On Fri, 28 Jun 2019 14:39:31 -0700, Shannon Nelson wrote:
> Add in the detailed statistics for ethtool -S that the driver
> keeps as it processes packets. Display of the additional
> debug statistics can be enabled through the ethtool priv-flags
> feature.
>
> Signed-off-by: Shannon Nelson <snelson@pensando.io>
> diff --git a/drivers/net/ethernet/pensando/ionic/Makefile b/drivers/net/ethernet/pensando/ionic/Makefile
> index 0e2dc53f08d4..4f3cfbf36c23 100644
> --- a/drivers/net/ethernet/pensando/ionic/Makefile
> +++ b/drivers/net/ethernet/pensando/ionic/Makefile
> @@ -4,4 +4,5 @@
> obj-$(CONFIG_IONIC) := ionic.o
>
> ionic-y := ionic_main.o ionic_bus_pci.o ionic_dev.o ionic_ethtool.o \
> - ionic_lif.o ionic_rx_filter.o ionic_txrx.o ionic_debugfs.o
> + ionic_lif.o ionic_rx_filter.o ionic_txrx.o ionic_debugfs.o \
> + ionic_stats.o
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
> index 2bbe5819387b..518e79c90fca 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
> @@ -8,6 +8,84 @@
> #include "ionic_bus.h"
> #include "ionic_lif.h"
> #include "ionic_ethtool.h"
> +#include "ionic_stats.h"
> +
> +static const char ionic_priv_flags_strings[][ETH_GSTRING_LEN] = {
> +#define PRIV_F_SW_DBG_STATS BIT(0)
> + "sw-dbg-stats",
> +};
> +#define PRIV_FLAGS_COUNT ARRAY_SIZE(ionic_priv_flags_strings)
> +
> +static void ionic_get_stats_strings(struct lif *lif, u8 *buf)
> +{
> + u32 i;
> +
> + for (i = 0; i < ionic_num_stats_grps; i++)
> + ionic_stats_groups[i].get_strings(lif, &buf);
> +}
> +
> +static void ionic_get_stats(struct net_device *netdev,
> + struct ethtool_stats *stats, u64 *buf)
> +{
> + struct lif *lif;
> + u32 i;
> +
> + lif = netdev_priv(netdev);
> +
> + memset(buf, 0, stats->n_stats * sizeof(*buf));
> + for (i = 0; i < ionic_num_stats_grps; i++)
> + ionic_stats_groups[i].get_values(lif, &buf);
> +}
> +
> +static int ionic_get_stats_count(struct lif *lif)
> +{
> + int i, num_stats = 0;
> +
> + for (i = 0; i < ionic_num_stats_grps; i++)
> + num_stats += ionic_stats_groups[i].get_count(lif);
> +
> + return num_stats;
> +}
> +
> +static int ionic_get_sset_count(struct net_device *netdev, int sset)
> +{
> + struct lif *lif = netdev_priv(netdev);
> + int count = 0;
> +
> + switch (sset) {
> + case ETH_SS_STATS:
> + count = ionic_get_stats_count(lif);
> + break;
> + case ETH_SS_TEST:
> + break;
> + case ETH_SS_PRIV_FLAGS:
> + count = PRIV_FLAGS_COUNT;
> + break;
> + default:
> + return -EOPNOTSUPP;
> + }
> + return count;
> +}
> +
> +static void ionic_get_strings(struct net_device *netdev,
> + u32 sset, u8 *buf)
> +{
> + struct lif *lif = netdev_priv(netdev);
> +
> + switch (sset) {
> + case ETH_SS_STATS:
> + ionic_get_stats_strings(lif, buf);
> + break;
> + case ETH_SS_PRIV_FLAGS:
> + memcpy(buf, ionic_priv_flags_strings,
> + PRIV_FLAGS_COUNT * ETH_GSTRING_LEN);
> + break;
> + case ETH_SS_TEST:
> + // IONIC_TODO
> + default:
> + netdev_err(netdev, "Invalid sset %d\n", sset);
Not really an error, as long as sset_count() returns a 0 nothing will
happen. Also you can drop the SS_TEST if you don't report it.
> + }
> +}
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_stats.h b/drivers/net/ethernet/pensando/ionic/ionic_stats.h
> new file mode 100644
> index 000000000000..b5487e7fd4fb
> --- /dev/null
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_stats.h
> @@ -0,0 +1,53 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright(c) 2017 - 2019 Pensando Systems, Inc */
> +
> +#ifndef _IONIC_STATS_H_
> +#define _IONIC_STATS_H_
> +
> +#define IONIC_STAT_TO_OFFSET(type, stat_name) (offsetof(type, stat_name))
> +
> +#define IONIC_STAT_DESC(type, stat_name) { \
> + .name = #stat_name, \
> + .offset = IONIC_STAT_TO_OFFSET(type, stat_name) \
> +}
> +
> +#define IONIC_LIF_STAT_DESC(stat_name) \
> + IONIC_STAT_DESC(struct lif_sw_stats, stat_name)
> +
> +#define IONIC_TX_STAT_DESC(stat_name) \
> + IONIC_STAT_DESC(struct tx_stats, stat_name)
> +
> +#define IONIC_RX_STAT_DESC(stat_name) \
> + IONIC_STAT_DESC(struct rx_stats, stat_name)
> +
> +#define IONIC_TX_Q_STAT_DESC(stat_name) \
> + IONIC_STAT_DESC(struct queue, stat_name)
> +
> +#define IONIC_CQ_STAT_DESC(stat_name) \
> + IONIC_STAT_DESC(struct cq, stat_name)
> +
> +#define IONIC_INTR_STAT_DESC(stat_name) \
> + IONIC_STAT_DESC(struct intr, stat_name)
> +
> +#define IONIC_NAPI_STAT_DESC(stat_name) \
> + IONIC_STAT_DESC(struct napi_stats, stat_name)
> +
> +/* Interface structure for a particalar stats group */
> +struct ionic_stats_group_intf {
> + void (*get_strings)(struct lif *lif, u8 **buf);
> + void (*get_values)(struct lif *lif, u64 **buf);
> + u64 (*get_count)(struct lif *lif);
> +};
> +
> +extern const struct ionic_stats_group_intf ionic_stats_groups[];
> +extern const int ionic_num_stats_grps;
> +
> +#define IONIC_READ_STAT64(base_ptr, desc_ptr) \
> + (*((u64 *)(((u8 *)(base_ptr)) + (desc_ptr)->offset)))
> +
> +struct ionic_stat_desc {
> + char name[ETH_GSTRING_LEN];
> + u64 offset;
> +};
> +
> +#endif // _IONIC_STATS_H_
Perhaps worth grepping the driver for C++ style comments?
^ permalink raw reply
* Re: [PATCH v2 net-next 14/19] ionic: Add Tx and Rx handling
From: Jakub Kicinski @ 2019-06-29 18:57 UTC (permalink / raw)
To: Shannon Nelson; +Cc: netdev
In-Reply-To: <20190628213934.8810-15-snelson@pensando.io>
On Fri, 28 Jun 2019 14:39:29 -0700, Shannon Nelson wrote:
> +static int ionic_tx(struct queue *q, struct sk_buff *skb)
> +{
> + struct tx_stats *stats = q_to_tx_stats(q);
> + int err;
> +
> + if (skb->ip_summed == CHECKSUM_PARTIAL)
> + err = ionic_tx_calc_csum(q, skb);
> + else
> + err = ionic_tx_calc_no_csum(q, skb);
> + if (err)
> + return err;
> +
> + err = ionic_tx_skb_frags(q, skb);
> + if (err)
> + return err;
> +
> + skb_tx_timestamp(skb);
> + stats->pkts++;
> + stats->bytes += skb->len;
Presumably this is 64bit so you should use
u64_stats_update_begin()
u64_stats_update_end()
around it (and all other stats).
> +
> + ionic_txq_post(q, !netdev_xmit_more(), ionic_tx_clean, skb);
> +
> + return 0;
> +}
> +
> +static int ionic_tx_descs_needed(struct queue *q, struct sk_buff *skb)
> +{
> + struct tx_stats *stats = q_to_tx_stats(q);
> + int err;
> +
> + /* If TSO, need roundup(skb->len/mss) descs */
> + if (skb_is_gso(skb))
> + return (skb->len / skb_shinfo(skb)->gso_size) + 1;
> +
> + /* If non-TSO, just need 1 desc and nr_frags sg elems */
> + if (skb_shinfo(skb)->nr_frags <= IONIC_TX_MAX_SG_ELEMS)
> + return 1;
> +
> + /* Too many frags, so linearize */
> + err = skb_linearize(skb);
> + if (err)
> + return err;
> +
> + stats->linearize++;
> +
> + /* Need 1 desc and zero sg elems */
> + return 1;
> +}
> +
> +netdev_tx_t ionic_start_xmit(struct sk_buff *skb, struct net_device *netdev)
> +{
> + u16 queue_index = skb_get_queue_mapping(skb);
> + struct lif *lif = netdev_priv(netdev);
> + struct queue *q;
> + int ndescs;
> + int err;
> +
> + if (unlikely(!test_bit(LIF_UP, lif->state))) {
> + dev_kfree_skb(skb);
> + return NETDEV_TX_OK;
> + }
> +
> + if (likely(lif_to_txqcq(lif, queue_index)))
> + q = lif_to_txq(lif, queue_index);
> + else
> + q = lif_to_txq(lif, 0);
> +
> + ndescs = ionic_tx_descs_needed(q, skb);
> + if (ndescs < 0)
> + goto err_out_drop;
> +
> + if (!ionic_q_has_space(q, ndescs)) {
> + netif_stop_subqueue(netdev, queue_index);
> + q->stop++;
> +
> + /* Might race with ionic_tx_clean, check again */
> + smp_rmb();
> + if (ionic_q_has_space(q, ndescs)) {
> + netif_wake_subqueue(netdev, queue_index);
> + q->wake++;
> + } else {
> + return NETDEV_TX_BUSY;
This should never really happen..
> + }
> + }
> +
> + if (skb_is_gso(skb))
> + err = ionic_tx_tso(q, skb);
> + else
> + err = ionic_tx(q, skb);
> +
> + if (err)
> + goto err_out_drop;
.. at this point if you can't guarantee fitting biggest possible frame
in, you have to stop the ring.
> + return NETDEV_TX_OK;
> +
> +err_out_drop:
> + q->drop++;
> + dev_kfree_skb(skb);
> + return NETDEV_TX_OK;
> +}
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox