From: Simon Horman <simon.horman@netronome.com>
To: Jiri Pirko <jiri@resnulli.us>, Scott Feldman <sfeldma@gmail.com>,
David Miller <davem@davemloft.net>
Cc: netdev@vger.kernel.org, Simon Horman <simon.horman@netronome.com>
Subject: [PATCH net-next 2/4] rocker: do not modify fdb table in rocker_port_fdb() when preparing transactions
Date: Tue, 19 May 2015 15:24:15 +0900 [thread overview]
Message-ID: <1432016657-24231-3-git-send-email-simon.horman@netronome.com> (raw)
In-Reply-To: <1432016657-24231-1-git-send-email-simon.horman@netronome.com>
rocker_port_fdb_flush() may be called be called with
trans == SWITCHDEV_TRANS_PREPARE and then trans == SWITCHDEV_TRANS_COMMIT from
switchdev_port_attr_set() via switchdev_port_obj_add().
Adding the new entry to the FDB table when trans == SWITCHDEV_TRANS_PREPARE
may result in a memory leak because when trans == SWITCHDEV_TRANS_PREPARE
rocker_flow_tbl_bridge() will allocate memory when called via
rocker_port_fdb_learn(). However, when trans == SWITCHDEV_TRANS_COMMIT
the presence of the FDB entry in the FDB table causes
rocker_port_fdb() to set the ROCKER_OP_FLAG_REFRESH flag which results
in rocker_port_fdb_learn() skipping the call to rocker_flow_tbl_bridge()
which would free the memory allocated by it when
trans == SWITCHDEV_TRANS_PREPARE.
ip link add br0 type bridge
ip link set up dev eth0
ip link set dev eth0 master br0
bridge fdb add 52:54:00:12:35:08 dev eth0
bridge fdb add 52:54:00:12:35:09 dev eth0
[ 2.600730] ------------[ cut here ]------------
[ 2.601002] kernel BUG at drivers/net/ethernet/rocker/rocker.c:4369!
[ 2.601373] invalid opcode: 0000 [#1] SMP
[ 2.601963] Modules linked in:
[ 2.602355] CPU: 0 PID: 64 Comm: bridge Not tainted 4.1.0-rc3-01048-g6d0f50c50211-dirty #1075
[ 2.602721] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.8.0-0-g4c59f5d-20150219_092859-nilsson.home.kraxel.org 04/01/2014
[ 2.602721] task: ffff880019facef0 ti: ffff88001f96c000 task.ti: ffff88001f96c000
[ 2.602721] RIP: 0010:[<ffffffff811f1470>] [<ffffffff811f1470>] rocker_port_obj_add+0x150/0x160
[ 2.602721] RSP: 0018:ffff88001f96fa98 EFLAGS: 00000212
[ 2.602721] RAX: ffff880019d4fa68 RBX: ffff88001f96fb18 RCX: 0000000000000000
[ 2.602721] RDX: ffff880019d4f000 RSI: ffff88001f96fb18 RDI: ffff880019d4f000
[ 2.602721] RBP: 0000000000000001 R08: 0000000000000000 R09: ffff88001f904620
[ 2.602721] R10: ffff88001f96fb60 R11: ffff880019e9d100 R12: ffff88001f96fb18
[ 2.602721] R13: ffff880019d4f680 R14: ffff88001f904610 R15: ffff8800198f7b80
[ 2.602721] FS: 00007f3eee917700(0000) GS:ffff88001b000000(0000) knlGS:0000000000000000
[ 2.602721] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 2.602721] CR2: 00007f3eee4a15cb CR3: 000000001f933000 CR4: 00000000000006b0
[ 2.602721] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 2.602721] DR3: 0000000000000000 DR6: 0000000000000000 DR7: 0000000000000000
[ 2.602721] Stack:
[ 2.602721] 0000000000000000 ffff88001f96fb18 ffff880019d4f000 ffff88001f96fb18
[ 2.602721] ffff880019d4f000 ffffffff81332105 ffff88001f96fb50 ffffffff814464c0
[ 2.602721] ffff88001f96fb18 ffff88001f904600 ffff880019d4f000 ffffffff813326e5
[ 2.602721] Call Trace:
[ 2.602721] [<ffffffff81332105>] ? __switchdev_port_obj_add+0x25/0x90
[ 2.602721] [<ffffffff813326e5>] ? switchdev_port_obj_add+0x25/0xc0
[ 2.602721] [<ffffffff813327b1>] ? switchdev_port_fdb_add+0x31/0x40
[ 2.602721] [<ffffffff8123911f>] ? rtnl_fdb_add+0xff/0x1e0
[ 2.602721] [<ffffffff81237d8e>] ? rtnetlink_rcv_msg+0x7e/0x250
[ 2.602721] [<ffffffff8121d1ce>] ? __skb_recv_datagram+0xfe/0x4b0
[ 2.602721] [<ffffffff81237d10>] ? rtnetlink_rcv+0x30/0x30
[ 2.602721] [<ffffffff81247958>] ? netlink_rcv_skb+0xa8/0xd0
[ 2.602721] [<ffffffff81237cff>] ? rtnetlink_rcv+0x1f/0x30
[ 2.602721] [<ffffffff81247220>] ? netlink_unicast+0x150/0x200
[ 2.602721] [<ffffffff81247714>] ? netlink_sendmsg+0x374/0x3e0
[ 2.602721] [<ffffffff8120f8df>] ? sock_sendmsg+0xf/0x30
[ 2.602721] [<ffffffff8120ffd3>] ? ___sys_sendmsg+0x1f3/0x200
[ 2.602721] [<ffffffff812100e5>] ? ___sys_recvmsg+0x105/0x140
[ 2.602721] [<ffffffff810a36f0>] ? SyS_readahead+0x90/0x90
[ 2.602721] [<ffffffff81098dfd>] ? filemap_map_pages+0x1ed/0x210
[ 2.602721] [<ffffffff810b77fc>] ? handle_mm_fault+0x5fc/0xe50
[ 2.602721] [<ffffffff81210ef9>] ? __sys_sendmsg+0x39/0x70
[ 2.602721] [<ffffffff8133ce17>] ? system_call_fastpath+0x12/0x6a
[ 2.602721] Code: b7 8f a0 06 00 00 48 83 bf 88 06 00 00 00 74 1d 48 83 c4 08 89 ee 4c 89 ef 5b 5d 41 5c 41 5d 0f b7 c9 45 31 c0 e9 51 db ff ff 90 <0f> 0b b8 ea ff ff ff e9 cf fe ff ff 0f 1f 40 00 41 57 41 56 b9
[ 2.602721] RIP [<ffffffff811f1470>] rocker_port_obj_add+0x150/0x160
[ 2.602721] RSP <ffff88001f96fa98>
[ 2.615848] ---[ end trace 4f7b4f1c98077108 ]---
The above is resolved by not adding the new FDB entry to the FDB table
if trans == SWITCHDEV_TRANS_PREPARE.
For symmetry this patch also skips deleting FDB entries from the FDB
table trans == SWITCHDEV_TRANS_PREPARE. However, my analysis is that
this never occurs as trans is always SWITCHDEV_TRANS_NONE when removing
FDB entries.
Fixes: c4f20321d968 ("rocker: support prepare-commit transaction model")
Signed-off-by: Simon Horman <simon.horman@netronome.com>
---
drivers/net/ethernet/rocker/rocker.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/rocker/rocker.c b/drivers/net/ethernet/rocker/rocker.c
index ce1bfab077a7..89d22bdcbdc4 100644
--- a/drivers/net/ethernet/rocker/rocker.c
+++ b/drivers/net/ethernet/rocker/rocker.c
@@ -3612,9 +3612,11 @@ static int rocker_port_fdb(struct rocker_port *rocker_port,
if (removing && found) {
rocker_port_kfree(rocker_port, trans, fdb);
- hash_del(&found->entry);
+ if (trans != SWITCHDEV_TRANS_PREPARE)
+ hash_del(&found->entry);
} else if (!removing && !found) {
- hash_add(rocker->fdb_tbl, &fdb->entry, fdb->key_crc32);
+ if (trans != SWITCHDEV_TRANS_PREPARE)
+ hash_add(rocker->fdb_tbl, &fdb->entry, fdb->key_crc32);
}
spin_unlock_irqrestore(&rocker->fdb_tbl_lock, lock_flags);
--
2.1.4
next prev parent reply other threads:[~2015-05-19 6:24 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-05-19 6:24 [PATCH net-next 0/4] rocker: transaction fixes Simon Horman
2015-05-19 6:24 ` [PATCH net-next 1/4] rocker: do not delete fdb entries in rocker_port_fdb_flush() when preparing transactions Simon Horman
2015-05-19 15:57 ` Scott Feldman
2015-05-19 6:24 ` Simon Horman [this message]
2015-05-19 16:01 ` [PATCH net-next 2/4] rocker: do not modify fdb table in rocker_port_fdb() " Scott Feldman
2015-05-19 6:24 ` [PATCH net-next 3/4] rocker: do not make neighbour entry changes " Simon Horman
2015-05-19 8:32 ` Toshiaki Makita
2015-05-20 1:53 ` Simon Horman
2015-05-19 16:11 ` Scott Feldman
2015-05-19 22:33 ` Simon Horman
2015-05-19 6:24 ` [PATCH net-next 4/4] rocker: make rocker_port_internal_vlan_id_{get,put}() non-transactional Simon Horman
2015-05-19 16:04 ` Scott Feldman
2015-05-19 16:13 ` [PATCH net-next 0/4] rocker: transaction fixes Scott Feldman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1432016657-24231-3-git-send-email-simon.horman@netronome.com \
--to=simon.horman@netronome.com \
--cc=davem@davemloft.net \
--cc=jiri@resnulli.us \
--cc=netdev@vger.kernel.org \
--cc=sfeldma@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).