Commit 4ac2dc89 authored by David S. Miller's avatar David S. Miller

Merge branch 'rocker-transaction-fixes'

Simon Horman says:

====================
rocker: transaction fixes

this series addresses what appear to be errors in the handling of
prepare and then commit transactions in the rocker driver.

In all cases the problem is that data structures visible outside of
the transaction are modified during the prepare phase.

In the case of the first two patches this results in the kernel reporting a
BUG. I have noted test-cases in the change logs.

The third patch is also a bug fix, as noted by  Toshiaki Makita,
however I have not been able to reliably reproduce the problem and
thus have not provided a test case.

The last patch is a correctness fix that does not fix a bug
that manifests as far as I can tell.

Changes: v3->v4
* All patches
  - Add Jiri Pirko's ack
* "rocker: do not make neighbour entry changes when preparing transactions"
  - Setting of entry values in all transaction phases
    as suggested by Toshiaki Makita
* "rocker: make rocker_port_internal_vlan_id_{get,put}() non-transactional"
  - Remove Fixes tag as I believe this is a correctness rather than a bug fix

Changes: v2->v3
* "rocker: do not make neighbour entry changes when preparing transactions"
  - Correct inverted logic
  - Added ack from Scott Feldman

Changes: v1->v2
* "rocker: do not make neighbour entry changes when preparing transactions"
  - Revised changelog to reflect information from Toshiaki Makita
    that there is a bug that can manifest
  - Update address and ttl regardless of the value of the transaction state
* All other patches
  - Added acks from Scott Feldman
====================
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parents e26cc7ff df6a2067
...@@ -2909,9 +2909,13 @@ static struct rocker_neigh_tbl_entry * ...@@ -2909,9 +2909,13 @@ static struct rocker_neigh_tbl_entry *
} }
static void _rocker_neigh_add(struct rocker *rocker, static void _rocker_neigh_add(struct rocker *rocker,
enum switchdev_trans trans,
struct rocker_neigh_tbl_entry *entry) struct rocker_neigh_tbl_entry *entry)
{ {
entry->index = rocker->neigh_tbl_next_index++; entry->index = rocker->neigh_tbl_next_index;
if (trans == SWITCHDEV_TRANS_PREPARE)
return;
rocker->neigh_tbl_next_index++;
entry->ref_count++; entry->ref_count++;
hash_add(rocker->neigh_tbl, &entry->entry, hash_add(rocker->neigh_tbl, &entry->entry,
be32_to_cpu(entry->ip_addr)); be32_to_cpu(entry->ip_addr));
...@@ -2921,6 +2925,8 @@ static void _rocker_neigh_del(struct rocker_port *rocker_port, ...@@ -2921,6 +2925,8 @@ static void _rocker_neigh_del(struct rocker_port *rocker_port,
enum switchdev_trans trans, enum switchdev_trans trans,
struct rocker_neigh_tbl_entry *entry) struct rocker_neigh_tbl_entry *entry)
{ {
if (trans == SWITCHDEV_TRANS_PREPARE)
return;
if (--entry->ref_count == 0) { if (--entry->ref_count == 0) {
hash_del(&entry->entry); hash_del(&entry->entry);
rocker_port_kfree(rocker_port, trans, entry); rocker_port_kfree(rocker_port, trans, entry);
...@@ -2928,12 +2934,13 @@ static void _rocker_neigh_del(struct rocker_port *rocker_port, ...@@ -2928,12 +2934,13 @@ static void _rocker_neigh_del(struct rocker_port *rocker_port,
} }
static void _rocker_neigh_update(struct rocker_neigh_tbl_entry *entry, static void _rocker_neigh_update(struct rocker_neigh_tbl_entry *entry,
enum switchdev_trans trans,
u8 *eth_dst, bool ttl_check) u8 *eth_dst, bool ttl_check)
{ {
if (eth_dst) { if (eth_dst) {
ether_addr_copy(entry->eth_dst, eth_dst); ether_addr_copy(entry->eth_dst, eth_dst);
entry->ttl_check = ttl_check; entry->ttl_check = ttl_check;
} else { } else if (trans != SWITCHDEV_TRANS_PREPARE) {
entry->ref_count++; entry->ref_count++;
} }
} }
...@@ -2973,12 +2980,12 @@ static int rocker_port_ipv4_neigh(struct rocker_port *rocker_port, ...@@ -2973,12 +2980,12 @@ static int rocker_port_ipv4_neigh(struct rocker_port *rocker_port,
entry->dev = rocker_port->dev; entry->dev = rocker_port->dev;
ether_addr_copy(entry->eth_dst, eth_dst); ether_addr_copy(entry->eth_dst, eth_dst);
entry->ttl_check = true; entry->ttl_check = true;
_rocker_neigh_add(rocker, entry); _rocker_neigh_add(rocker, trans, entry);
} else if (removing) { } else if (removing) {
memcpy(entry, found, sizeof(*entry)); memcpy(entry, found, sizeof(*entry));
_rocker_neigh_del(rocker_port, trans, found); _rocker_neigh_del(rocker_port, trans, found);
} else if (updating) { } else if (updating) {
_rocker_neigh_update(found, eth_dst, true); _rocker_neigh_update(found, trans, eth_dst, true);
memcpy(entry, found, sizeof(*entry)); memcpy(entry, found, sizeof(*entry));
} else { } else {
err = -ENOENT; err = -ENOENT;
...@@ -3089,13 +3096,13 @@ static int rocker_port_ipv4_nh(struct rocker_port *rocker_port, ...@@ -3089,13 +3096,13 @@ static int rocker_port_ipv4_nh(struct rocker_port *rocker_port,
if (adding) { if (adding) {
entry->ip_addr = ip_addr; entry->ip_addr = ip_addr;
entry->dev = rocker_port->dev; entry->dev = rocker_port->dev;
_rocker_neigh_add(rocker, entry); _rocker_neigh_add(rocker, trans, entry);
*index = entry->index; *index = entry->index;
resolved = false; resolved = false;
} else if (removing) { } else if (removing) {
_rocker_neigh_del(rocker_port, trans, found); _rocker_neigh_del(rocker_port, trans, found);
} else if (updating) { } else if (updating) {
_rocker_neigh_update(found, NULL, false); _rocker_neigh_update(found, trans, NULL, false);
resolved = !is_zero_ether_addr(found->eth_dst); resolved = !is_zero_ether_addr(found->eth_dst);
} else { } else {
err = -ENOENT; err = -ENOENT;
...@@ -3612,9 +3619,11 @@ static int rocker_port_fdb(struct rocker_port *rocker_port, ...@@ -3612,9 +3619,11 @@ static int rocker_port_fdb(struct rocker_port *rocker_port,
if (removing && found) { if (removing && found) {
rocker_port_kfree(rocker_port, trans, fdb); rocker_port_kfree(rocker_port, trans, fdb);
hash_del(&found->entry); if (trans != SWITCHDEV_TRANS_PREPARE)
hash_del(&found->entry);
} else if (!removing && !found) { } 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); spin_unlock_irqrestore(&rocker->fdb_tbl_lock, lock_flags);
...@@ -3658,7 +3667,8 @@ static int rocker_port_fdb_flush(struct rocker_port *rocker_port, ...@@ -3658,7 +3667,8 @@ static int rocker_port_fdb_flush(struct rocker_port *rocker_port,
found->key.vlan_id); found->key.vlan_id);
if (err) if (err)
goto err_out; goto err_out;
hash_del(&found->entry); if (trans != SWITCHDEV_TRANS_PREPARE)
hash_del(&found->entry);
} }
err_out: err_out:
...@@ -3843,7 +3853,6 @@ rocker_internal_vlan_tbl_find(struct rocker *rocker, int ifindex) ...@@ -3843,7 +3853,6 @@ rocker_internal_vlan_tbl_find(struct rocker *rocker, int ifindex)
} }
static __be16 rocker_port_internal_vlan_id_get(struct rocker_port *rocker_port, static __be16 rocker_port_internal_vlan_id_get(struct rocker_port *rocker_port,
enum switchdev_trans trans,
int ifindex) int ifindex)
{ {
struct rocker *rocker = rocker_port->rocker; struct rocker *rocker = rocker_port->rocker;
...@@ -3852,7 +3861,7 @@ static __be16 rocker_port_internal_vlan_id_get(struct rocker_port *rocker_port, ...@@ -3852,7 +3861,7 @@ static __be16 rocker_port_internal_vlan_id_get(struct rocker_port *rocker_port,
unsigned long lock_flags; unsigned long lock_flags;
int i; int i;
entry = rocker_port_kzalloc(rocker_port, trans, sizeof(*entry)); entry = kzalloc(sizeof(*entry), GFP_KERNEL);
if (!entry) if (!entry)
return 0; return 0;
...@@ -3862,7 +3871,7 @@ static __be16 rocker_port_internal_vlan_id_get(struct rocker_port *rocker_port, ...@@ -3862,7 +3871,7 @@ static __be16 rocker_port_internal_vlan_id_get(struct rocker_port *rocker_port,
found = rocker_internal_vlan_tbl_find(rocker, ifindex); found = rocker_internal_vlan_tbl_find(rocker, ifindex);
if (found) { if (found) {
rocker_port_kfree(rocker_port, trans, entry); kfree(entry);
goto found; goto found;
} }
...@@ -3886,7 +3895,6 @@ static __be16 rocker_port_internal_vlan_id_get(struct rocker_port *rocker_port, ...@@ -3886,7 +3895,6 @@ static __be16 rocker_port_internal_vlan_id_get(struct rocker_port *rocker_port,
} }
static void rocker_port_internal_vlan_id_put(struct rocker_port *rocker_port, static void rocker_port_internal_vlan_id_put(struct rocker_port *rocker_port,
enum switchdev_trans trans,
int ifindex) int ifindex)
{ {
struct rocker *rocker = rocker_port->rocker; struct rocker *rocker = rocker_port->rocker;
...@@ -3908,7 +3916,7 @@ static void rocker_port_internal_vlan_id_put(struct rocker_port *rocker_port, ...@@ -3908,7 +3916,7 @@ static void rocker_port_internal_vlan_id_put(struct rocker_port *rocker_port,
bit = ntohs(found->vlan_id) - ROCKER_INTERNAL_VLAN_ID_BASE; bit = ntohs(found->vlan_id) - ROCKER_INTERNAL_VLAN_ID_BASE;
clear_bit(bit, rocker->internal_vlan_bitmap); clear_bit(bit, rocker->internal_vlan_bitmap);
hash_del(&found->entry); hash_del(&found->entry);
rocker_port_kfree(rocker_port, trans, found); kfree(found);
} }
not_found: not_found:
...@@ -4894,9 +4902,7 @@ static int rocker_probe_port(struct rocker *rocker, unsigned int port_number) ...@@ -4894,9 +4902,7 @@ static int rocker_probe_port(struct rocker *rocker, unsigned int port_number)
rocker_port_set_learning(rocker_port, SWITCHDEV_TRANS_NONE); rocker_port_set_learning(rocker_port, SWITCHDEV_TRANS_NONE);
rocker_port->internal_vlan_id = rocker_port->internal_vlan_id =
rocker_port_internal_vlan_id_get(rocker_port, rocker_port_internal_vlan_id_get(rocker_port, dev->ifindex);
SWITCHDEV_TRANS_NONE,
dev->ifindex);
err = rocker_port_ig_tbl(rocker_port, SWITCHDEV_TRANS_NONE, 0); err = rocker_port_ig_tbl(rocker_port, SWITCHDEV_TRANS_NONE, 0);
if (err) { if (err) {
dev_err(&pdev->dev, "install ig port table failed\n"); dev_err(&pdev->dev, "install ig port table failed\n");
...@@ -5144,7 +5150,7 @@ static int rocker_port_bridge_join(struct rocker_port *rocker_port, ...@@ -5144,7 +5150,7 @@ static int rocker_port_bridge_join(struct rocker_port *rocker_port,
{ {
int err; int err;
rocker_port_internal_vlan_id_put(rocker_port, SWITCHDEV_TRANS_NONE, rocker_port_internal_vlan_id_put(rocker_port,
rocker_port->dev->ifindex); rocker_port->dev->ifindex);
rocker_port->bridge_dev = bridge; rocker_port->bridge_dev = bridge;
...@@ -5155,9 +5161,7 @@ static int rocker_port_bridge_join(struct rocker_port *rocker_port, ...@@ -5155,9 +5161,7 @@ static int rocker_port_bridge_join(struct rocker_port *rocker_port,
if (err) if (err)
return err; return err;
rocker_port->internal_vlan_id = rocker_port->internal_vlan_id =
rocker_port_internal_vlan_id_get(rocker_port, rocker_port_internal_vlan_id_get(rocker_port, bridge->ifindex);
SWITCHDEV_TRANS_NONE,
bridge->ifindex);
return rocker_port_vlan(rocker_port, SWITCHDEV_TRANS_NONE, 0, 0); return rocker_port_vlan(rocker_port, SWITCHDEV_TRANS_NONE, 0, 0);
} }
...@@ -5165,7 +5169,7 @@ static int rocker_port_bridge_leave(struct rocker_port *rocker_port) ...@@ -5165,7 +5169,7 @@ static int rocker_port_bridge_leave(struct rocker_port *rocker_port)
{ {
int err; int err;
rocker_port_internal_vlan_id_put(rocker_port, SWITCHDEV_TRANS_NONE, rocker_port_internal_vlan_id_put(rocker_port,
rocker_port->bridge_dev->ifindex); rocker_port->bridge_dev->ifindex);
rocker_port->bridge_dev = NULL; rocker_port->bridge_dev = NULL;
...@@ -5177,7 +5181,6 @@ static int rocker_port_bridge_leave(struct rocker_port *rocker_port) ...@@ -5177,7 +5181,6 @@ static int rocker_port_bridge_leave(struct rocker_port *rocker_port)
return err; return err;
rocker_port->internal_vlan_id = rocker_port->internal_vlan_id =
rocker_port_internal_vlan_id_get(rocker_port, rocker_port_internal_vlan_id_get(rocker_port,
SWITCHDEV_TRANS_NONE,
rocker_port->dev->ifindex); rocker_port->dev->ifindex);
err = rocker_port_vlan(rocker_port, SWITCHDEV_TRANS_NONE, 0, 0); err = rocker_port_vlan(rocker_port, SWITCHDEV_TRANS_NONE, 0, 0);
if (err) if (err)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment