Discussion:
[PATCH net v2 4/4] cxgb4i: Remove duplicate call to dst_neigh_lookup()
Anish Bhatt
2014-10-15 03:07:24 UTC
Permalink
There is an extra call to dst_neigh_lookup() leftover in cxgb4i that can cause
an unreleased refcnt issue. Remove extraneous call.

Signed-off-by: Anish Bhatt <***@chelsio.com>

Fixes : 759a0cc5a3e1b ('cxgb4i: Add ipv6 code to driver, call into libcxgbi ipv6 api')
---
drivers/scsi/cxgbi/cxgb4i/cxgb4i.c | 5 -----
1 file changed, 5 deletions(-)

diff --git a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
index df176f0..8c3003b 100644
--- a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
+++ b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
@@ -1317,11 +1317,6 @@ static int init_act_open(struct cxgbi_sock *csk)
cxgbi_sock_set_flag(csk, CTPF_HAS_ATID);
cxgbi_sock_get(csk);

- n = dst_neigh_lookup(csk->dst, &csk->daddr.sin_addr.s_addr);
- if (!n) {
- pr_err("%s, can't get neighbour of csk->dst.\n", ndev->name);
- goto rel_resource;
- }
csk->l2t = cxgb4_l2t_get(lldi->l2t, n, ndev, 0);
if (!csk->l2t) {
pr_err("%s, cannot alloc l2t.\n", ndev->name);
--
2.1.2
Anish Bhatt
2014-10-15 03:07:21 UTC
Permalink
cxgb4 already handles CLIP updates from a previous changeset for iw_cxgb4,
there is no need to have this functionality in cxgb4i. Remove duplicated code

Signed-off-by: Anish Bhatt <***@chelsio.com>
---
drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 7 ++
drivers/scsi/cxgbi/cxgb4i/cxgb4i.c | 133 ------------------------
2 files changed, 7 insertions(+), 133 deletions(-)

diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
index fed8f26..411acf0 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
@@ -4490,6 +4490,13 @@ static int update_root_dev_clip(struct net_device *dev)
return ret;

/* Parse all bond and vlan devices layered on top of the physical dev */
+ root_dev = netdev_master_upper_dev_get_rcu(dev);
+ if (root_dev) {
+ ret = update_dev_clip(root_dev, dev);
+ if (ret)
+ return ret;
+ }
+
for (i = 0; i < VLAN_N_VID; i++) {
root_dev = __vlan_find_dev_deep_rcu(dev, htons(ETH_P_8021Q), i);
if (!root_dev)
diff --git a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
index 02e69e7..18d0d1c 100644
--- a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
+++ b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
@@ -1635,129 +1635,6 @@ static int cxgb4i_ddp_init(struct cxgbi_device *cdev)
return 0;
}

-#if IS_ENABLED(CONFIG_IPV6)
-static int cxgbi_inet6addr_handler(struct notifier_block *this,
- unsigned long event, void *data)
-{
- struct inet6_ifaddr *ifa = data;
- struct net_device *event_dev = ifa->idev->dev;
- struct cxgbi_device *cdev;
- int ret = NOTIFY_DONE;
-
- if (event_dev->priv_flags & IFF_802_1Q_VLAN)
- event_dev = vlan_dev_real_dev(event_dev);
-
- cdev = cxgbi_device_find_by_netdev_rcu(event_dev, NULL);
-
- if (!cdev)
- return ret;
-
- switch (event) {
- case NETDEV_UP:
- ret = cxgb4_clip_get(event_dev,
- (const struct in6_addr *)
- ((ifa)->addr.s6_addr));
- if (ret < 0)
- return ret;
-
- ret = NOTIFY_OK;
- break;
-
- case NETDEV_DOWN:
- cxgb4_clip_release(event_dev,
- (const struct in6_addr *)
- ((ifa)->addr.s6_addr));
- ret = NOTIFY_OK;
- break;
-
- default:
- break;
- }
-
- return ret;
-}
-
-static struct notifier_block cxgbi_inet6addr_notifier = {
- .notifier_call = cxgbi_inet6addr_handler
-};
-
-/* Retrieve IPv6 addresses from a root device (bond, vlan) associated with
- * a physical device.
- * The physical device reference is needed to send the actual CLIP command.
- */
-static int update_dev_clip(struct net_device *root_dev, struct net_device *dev)
-{
- struct inet6_dev *idev = NULL;
- struct inet6_ifaddr *ifa;
- int ret = 0;
-
- idev = __in6_dev_get(root_dev);
- if (!idev)
- return ret;
-
- read_lock_bh(&idev->lock);
- list_for_each_entry(ifa, &idev->addr_list, if_list) {
- pr_info("updating the clip for addr %pI6\n",
- ifa->addr.s6_addr);
- ret = cxgb4_clip_get(dev, (const struct in6_addr *)
- ifa->addr.s6_addr);
- if (ret < 0)
- break;
- }
-
- read_unlock_bh(&idev->lock);
- return ret;
-}
-
-static int update_root_dev_clip(struct net_device *dev)
-{
- struct net_device *root_dev = NULL;
- int i, ret = 0;
-
- /* First populate the real net device's IPv6 address */
- ret = update_dev_clip(dev, dev);
- if (ret)
- return ret;
-
- /* Parse all bond and vlan devices layered on top of the physical dev */
- root_dev = netdev_master_upper_dev_get(dev);
- if (root_dev) {
- ret = update_dev_clip(root_dev, dev);
- if (ret)
- return ret;
- }
-
- for (i = 0; i < VLAN_N_VID; i++) {
- root_dev = __vlan_find_dev_deep_rcu(dev, htons(ETH_P_8021Q), i);
- if (!root_dev)
- continue;
-
- ret = update_dev_clip(root_dev, dev);
- if (ret)
- break;
- }
- return ret;
-}
-
-static void cxgbi_update_clip(struct cxgbi_device *cdev)
-{
- int i;
-
- rcu_read_lock();
-
- for (i = 0; i < cdev->nports; i++) {
- struct net_device *dev = cdev->ports[i];
- int ret = 0;
-
- if (dev)
- ret = update_root_dev_clip(dev);
- if (ret < 0)
- break;
- }
- rcu_read_unlock();
-}
-#endif /* IS_ENABLED(CONFIG_IPV6) */
-
static void *t4_uld_add(const struct cxgb4_lld_info *lldi)
{
struct cxgbi_device *cdev;
@@ -1876,10 +1753,6 @@ static int t4_uld_state_change(void *handle, enum cxgb4_state state)
switch (state) {
case CXGB4_STATE_UP:
pr_info("cdev 0x%p, UP.\n", cdev);
-#if IS_ENABLED(CONFIG_IPV6)
- cxgbi_update_clip(cdev);
-#endif
- /* re-initialize */
break;
case CXGB4_STATE_START_RECOVERY:
pr_info("cdev 0x%p, RECOVERY.\n", cdev);
@@ -1910,17 +1783,11 @@ static int __init cxgb4i_init_module(void)
return rc;
cxgb4_register_uld(CXGB4_ULD_ISCSI, &cxgb4i_uld_info);

-#if IS_ENABLED(CONFIG_IPV6)
- register_inet6addr_notifier(&cxgbi_inet6addr_notifier);
-#endif
return 0;
}

static void __exit cxgb4i_exit_module(void)
{
-#if IS_ENABLED(CONFIG_IPV6)
- unregister_inet6addr_notifier(&cxgbi_inet6addr_notifier);
-#endif
cxgb4_unregister_uld(CXGB4_ULD_ISCSI);
cxgbi_device_unregister_all(CXGBI_FLAG_DEV_T4);
cxgbi_iscsi_cleanup(&cxgb4i_iscsi_transport, &cxgb4i_stt);
--
2.1.2
Anish Bhatt
2014-10-15 03:07:23 UTC
Permalink
A bunch of ipv6 related code is left on by default. While this causes no
compilation issues, there is no need to have this enabled by default. Guard
with an ipv6 check, which also takes care of a -Wunused-function warning.

Signed-off-by: Anish Bhatt <***@chelsio.com>
---
drivers/scsi/cxgbi/cxgb4i/cxgb4i.c | 8 ++++++++
drivers/scsi/cxgbi/libcxgbi.c | 2 ++
2 files changed, 10 insertions(+)

diff --git a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
index 18d0d1c..df176f0 100644
--- a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
+++ b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
@@ -259,6 +259,7 @@ static void send_act_open_req(struct cxgbi_sock *csk, struct sk_buff *skb,
cxgb4_l2t_send(csk->cdev->ports[csk->port_id], skb, csk->l2t);
}

+#if IS_ENABLED(CONFIG_IPV6)
static void send_act_open_req6(struct cxgbi_sock *csk, struct sk_buff *skb,
struct l2t_entry *e)
{
@@ -344,6 +345,7 @@ static void send_act_open_req6(struct cxgbi_sock *csk, struct sk_buff *skb,

cxgb4_l2t_send(csk->cdev->ports[csk->port_id], skb, csk->l2t);
}
+#endif

static void send_close_req(struct cxgbi_sock *csk)
{
@@ -781,9 +783,11 @@ static void csk_act_open_retry_timer(unsigned long data)
if (csk->csk_family == AF_INET) {
send_act_open_func = send_act_open_req;
skb = alloc_wr(size, 0, GFP_ATOMIC);
+#if IS_ENABLED(CONFIG_IPV6)
} else {
send_act_open_func = send_act_open_req6;
skb = alloc_wr(size6, 0, GFP_ATOMIC);
+#endif
}

if (!skb)
@@ -1335,8 +1339,10 @@ static int init_act_open(struct cxgbi_sock *csk)

if (csk->csk_family == AF_INET)
skb = alloc_wr(size, 0, GFP_NOIO);
+#if IS_ENABLED(CONFIG_IPV6)
else
skb = alloc_wr(size6, 0, GFP_NOIO);
+#endif

if (!skb)
goto rel_resource;
@@ -1370,8 +1376,10 @@ static int init_act_open(struct cxgbi_sock *csk)
cxgbi_sock_set_state(csk, CTP_ACTIVE_OPEN);
if (csk->csk_family == AF_INET)
send_act_open_req(csk, skb, csk->l2t);
+#if IS_ENABLED(CONFIG_IPV6)
else
send_act_open_req6(csk, skb, csk->l2t);
+#endif
neigh_release(n);

return 0;
diff --git a/drivers/scsi/cxgbi/libcxgbi.c b/drivers/scsi/cxgbi/libcxgbi.c
index 6a2001d..54fa6e0 100644
--- a/drivers/scsi/cxgbi/libcxgbi.c
+++ b/drivers/scsi/cxgbi/libcxgbi.c
@@ -275,6 +275,7 @@ struct cxgbi_device *cxgbi_device_find_by_netdev_rcu(struct net_device *ndev,
}
EXPORT_SYMBOL_GPL(cxgbi_device_find_by_netdev_rcu);

+#if IS_ENABLED(CONFIG_IPV6)
static struct cxgbi_device *cxgbi_device_find_by_mac(struct net_device *ndev,
int *port)
{
@@ -307,6 +308,7 @@ static struct cxgbi_device *cxgbi_device_find_by_mac(struct net_device *ndev,
ndev, ndev->name);
return NULL;
}
+#endif

void cxgbi_hbas_remove(struct cxgbi_device *cdev)
{
--
2.1.2
Anish Bhatt
2014-10-15 03:07:22 UTC
Permalink
cxgb4 ipv6 does not guard against ipv6 being disabled, or the standard
ipv6 module vs inbuilt tri-state issue. This was fixed for cxgb4i & iw_cxgb4
but missed for cxgb4.

Signed-off-by: Anish Bhatt <***@chelsio.com>
---
drivers/net/ethernet/chelsio/Kconfig | 2 +-
drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 8 ++++++++
2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/chelsio/Kconfig b/drivers/net/ethernet/chelsio/Kconfig
index c3ce9df..ac6473f 100644
--- a/drivers/net/ethernet/chelsio/Kconfig
+++ b/drivers/net/ethernet/chelsio/Kconfig
@@ -68,7 +68,7 @@ config CHELSIO_T3

config CHELSIO_T4
tristate "Chelsio Communications T4/T5 Ethernet support"
- depends on PCI
+ depends on PCI && (IPV6 || IPV6=n)
select FW_LOADER
select MDIO
---help---
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
index 411acf0..3f60070 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
@@ -4369,6 +4369,7 @@ EXPORT_SYMBOL(cxgb4_unregister_uld);
* success (true) if it belongs otherwise failure (false).
* Called with rcu_read_lock() held.
*/
+#if IS_ENABLED(CONFIG_IPV6)
static bool cxgb4_netdev(const struct net_device *netdev)
{
struct adapter *adap;
@@ -4529,6 +4530,7 @@ static void update_clip(const struct adapter *adap)
}
rcu_read_unlock();
}
+#endif /* IS_ENABLED(CONFIG_IPV6) */

/**
* cxgb_up - enable the adapter
@@ -4575,7 +4577,9 @@ static int cxgb_up(struct adapter *adap)
t4_intr_enable(adap);
adap->flags |= FULL_INIT_DONE;
notify_ulds(adap, CXGB4_STATE_UP);
+#if IS_ENABLED(CONFIG_IPV6)
update_clip(adap);
+#endif
out:
return err;
irq_err:
@@ -6869,14 +6873,18 @@ static int __init cxgb4_init_module(void)
if (ret < 0)
debugfs_remove(cxgb4_debugfs_root);

+#if IS_ENABLED(CONFIG_IPV6)
register_inet6addr_notifier(&cxgb4_inet6addr_notifier);
+#endif

return ret;
}

static void __exit cxgb4_cleanup_module(void)
{
+#if IS_ENABLED(CONFIG_IPV6)
unregister_inet6addr_notifier(&cxgb4_inet6addr_notifier);
+#endif
pci_unregister_driver(&cxgb4_driver);
debugfs_remove(cxgb4_debugfs_root); /* NULL ok */
}
--
2.1.2

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
David Miller
2014-10-15 04:30:48 UTC
Permalink
From: Anish Bhatt <***@chelsio.com>
Date: Tue, 14 Oct 2014 20:07:20 -0700
This patch set removes some duplicated/extraneous code from cxgb4i, guards
cxgb4 against compilation failure based on ipv6 tristate, make ipv6 related
code no longer be enabled by default irrespective of ipv6 tristate and fixes
a refcnt issue.
-Anish
v2 : Provide more detailed commit messages, make subject more concise as
recommended by Dave Miller.
Much better, series applied, thanks!

Loading...