* [PATCH net 1/5] bnxt_en: Disable bus master during PCI shutdown and driver unload.
From: Michael Chan @ 2019-06-29 15:16 UTC (permalink / raw)
To: davem; +Cc: netdev
In-Reply-To: <1561821408-17418-1-git-send-email-michael.chan@broadcom.com>
Some chips with older firmware can continue to perform DMA read from
context memory even after the memory has been freed. In the PCI shutdown
method, we need to call pci_disable_device() to shutdown DMA to prevent
this DMA before we put the device into D3hot. DMA memory request in
D3hot state will generate PCI fatal error. Similarly, in the driver
remove method, the context memory should only be freed after DMA has
been shutdown for correctness.
Fixes: 98f04cf0f1fc ("bnxt_en: Check context memory requirements from firmware.")
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index f758b2e..b9bc829 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -10262,10 +10262,10 @@ static void bnxt_remove_one(struct pci_dev *pdev)
bnxt_dcb_free(bp);
kfree(bp->edev);
bp->edev = NULL;
+ bnxt_cleanup_pci(bp);
bnxt_free_ctx_mem(bp);
kfree(bp->ctx);
bp->ctx = NULL;
- bnxt_cleanup_pci(bp);
bnxt_free_port_stats(bp);
free_netdev(dev);
}
@@ -10859,6 +10859,7 @@ static void bnxt_shutdown(struct pci_dev *pdev)
if (system_state == SYSTEM_POWER_OFF) {
bnxt_clear_int_mode(bp);
+ pci_disable_device(pdev);
pci_wake_from_d3(pdev, bp->wol);
pci_set_power_state(pdev, PCI_D3hot);
}
--
2.5.1
^ permalink raw reply related
* [PATCH net 2/5] bnxt_en: Fix ethtool selftest crash under error conditions.
From: Michael Chan @ 2019-06-29 15:16 UTC (permalink / raw)
To: davem; +Cc: netdev
In-Reply-To: <1561821408-17418-1-git-send-email-michael.chan@broadcom.com>
After ethtool loopback packet tests, we re-open the nic for the next
IRQ test. If the open fails, we must not proceed with the IRQ test
or we will crash with NULL pointer dereference. Fix it by checking
the bnxt_open_nic() return code before proceeding.
Reported-by: Somasundaram Krishnasamy <somasundaram.krishnasamy@oracle.com>
Fixes: 67fea463fd87 ("bnxt_en: Add interrupt test to ethtool -t selftest.")
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
index a6c7baf..ec68707 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
@@ -2842,7 +2842,7 @@ static void bnxt_self_test(struct net_device *dev, struct ethtool_test *etest,
bool offline = false;
u8 test_results = 0;
u8 test_mask = 0;
- int rc, i;
+ int rc = 0, i;
if (!bp->num_tests || !BNXT_SINGLE_PF(bp))
return;
@@ -2913,9 +2913,9 @@ static void bnxt_self_test(struct net_device *dev, struct ethtool_test *etest,
}
bnxt_hwrm_phy_loopback(bp, false, false);
bnxt_half_close_nic(bp);
- bnxt_open_nic(bp, false, true);
+ rc = bnxt_open_nic(bp, false, true);
}
- if (bnxt_test_irq(bp)) {
+ if (rc || bnxt_test_irq(bp)) {
buf[BNXT_IRQ_TEST_IDX] = 1;
etest->flags |= ETH_TEST_FL_FAILED;
}
--
2.5.1
^ permalink raw reply related
* [PATCH net 4/5] bnxt_en: Cap the returned MSIX vectors to the RDMA driver.
From: Michael Chan @ 2019-06-29 15:16 UTC (permalink / raw)
To: davem; +Cc: netdev
In-Reply-To: <1561821408-17418-1-git-send-email-michael.chan@broadcom.com>
In an earlier commit to improve NQ reservations on 57500 chips, we
set the resv_irqs on the 57500 VFs to the fixed value assigned by
the PF regardless of how many are actually used. The current
code assumes that resv_irqs minus the ones used by the network driver
must be the ones for the RDMA driver. This is no longer true and
we may return more MSIX vectors than requested, causing inconsistency.
Fix it by capping the value.
Fixes: 01989c6b69d9 ("bnxt_en: Improve NQ reservations.")
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
index bfa342a..fc77caf 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
@@ -157,8 +157,10 @@ static int bnxt_req_msix_vecs(struct bnxt_en_dev *edev, int ulp_id,
if (BNXT_NEW_RM(bp)) {
struct bnxt_hw_resc *hw_resc = &bp->hw_resc;
+ int resv_msix;
- avail_msix = hw_resc->resv_irqs - bp->cp_nr_rings;
+ resv_msix = hw_resc->resv_irqs - bp->cp_nr_rings;
+ avail_msix = min_t(int, resv_msix, avail_msix);
edev->ulp_tbl[ulp_id].msix_requested = avail_msix;
}
bnxt_fill_msix_vecs(bp, ent);
--
2.5.1
^ permalink raw reply related
* [PATCH net 5/5] bnxt_en: Suppress error messages when querying DSCP DCB capabilities.
From: Michael Chan @ 2019-06-29 15:16 UTC (permalink / raw)
To: davem; +Cc: netdev
In-Reply-To: <1561821408-17418-1-git-send-email-michael.chan@broadcom.com>
Some firmware versions do not support this so use the silent variant
to send the message to firmware to suppress the harmless error. This
error message is unnecessarily alarming the user.
Fixes: afdc8a84844a ("bnxt_en: Add DCBNL DSCP application protocol support.")
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c
index 7077515..07301cb 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c
@@ -396,7 +396,7 @@ static int bnxt_hwrm_queue_dscp_qcaps(struct bnxt *bp)
bnxt_hwrm_cmd_hdr_init(bp, &req, HWRM_QUEUE_DSCP_QCAPS, -1, -1);
mutex_lock(&bp->hwrm_cmd_lock);
- rc = _hwrm_send_message(bp, &req, sizeof(req), HWRM_CMD_TIMEOUT);
+ rc = _hwrm_send_message_silent(bp, &req, sizeof(req), HWRM_CMD_TIMEOUT);
if (!rc) {
bp->max_dscp_value = (1 << resp->num_dscp_bits) - 1;
if (bp->max_dscp_value < 0x3f)
--
2.5.1
^ permalink raw reply related
* [PATCH net 3/5] bnxt_en: Fix statistics context reservation logic for RDMA driver.
From: Michael Chan @ 2019-06-29 15:16 UTC (permalink / raw)
To: davem; +Cc: netdev
In-Reply-To: <1561821408-17418-1-git-send-email-michael.chan@broadcom.com>
The current logic assumes that the RDMA driver uses one statistics
context adjacent to the ones used by the network driver. This
assumption is not true and the statistics context used by the
RDMA driver is tied to its MSIX base vector. This wrong assumption
can cause RDMA driver failure after changing ethtool rings on the
network side. Fix the statistics reservation logic accordingly.
Fixes: 780baad44f0f ("bnxt_en: Reserve 1 stat_ctx for RDMA driver.")
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index b9bc829..9090c79 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -5508,7 +5508,16 @@ static int bnxt_cp_rings_in_use(struct bnxt *bp)
static int bnxt_get_func_stat_ctxs(struct bnxt *bp)
{
- return bp->cp_nr_rings + bnxt_get_ulp_stat_ctxs(bp);
+ int ulp_stat = bnxt_get_ulp_stat_ctxs(bp);
+ int cp = bp->cp_nr_rings;
+
+ if (!ulp_stat)
+ return cp;
+
+ if (bnxt_nq_rings_in_use(bp) > cp + bnxt_get_ulp_msix_num(bp))
+ return bnxt_get_ulp_msix_base(bp) + ulp_stat;
+
+ return cp + ulp_stat;
}
static bool bnxt_need_reserve_rings(struct bnxt *bp)
@@ -7477,11 +7486,7 @@ unsigned int bnxt_get_avail_cp_rings_for_en(struct bnxt *bp)
unsigned int bnxt_get_avail_stat_ctxs_for_en(struct bnxt *bp)
{
- unsigned int stat;
-
- stat = bnxt_get_max_func_stat_ctxs(bp) - bnxt_get_ulp_stat_ctxs(bp);
- stat -= bp->cp_nr_rings;
- return stat;
+ return bnxt_get_max_func_stat_ctxs(bp) - bnxt_get_func_stat_ctxs(bp);
}
int bnxt_get_avail_msix(struct bnxt *bp, int num)
--
2.5.1
^ permalink raw reply related
* 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
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