* Re: [PATCH iproute2 0/2] Use one func to output stats from Netlink and /proc
From: Vadim Kochan @ 2014-12-30 13:58 UTC (permalink / raw)
To: netdev; +Cc: Vadim Kochan
In-Reply-To: <1419897380-21012-1-git-send-email-vadim4j@gmail.com>
On Tue, Dec 30, 2014 at 01:56:18AM +0200, Vadim Kochan wrote:
> Refactoring to use one func to output sock stats
> from Netlink and /proc for unix and packet socket types.
>
> I did some testing but I might miss some cases, would be good if someone
> can test it too.
>
> Vadim Kochan (2):
> ss: Unify unix stats output from netlink and proc
> ss: Unify packet stats output from netlink and proc
>
> misc/ss.c | 310 ++++++++++++++++++++++++++------------------------------------
> 1 file changed, 132 insertions(+), 178 deletions(-)
>
> --
> 2.1.3
>
Please ignore this series as I have one more additional little fix which related to
these changes. The fix is related to issue when filtering does not work
for UNIX sockets when you do:
# ss -x src *X11*
Seems that the issue was started from:
(dfbaa90dec) iproute: Dump unix sockets via netlink
where filter was not considered in the netlink handler.
So I will re-send full series with using of filter for netlink diag handler too.
Regards,
^ permalink raw reply
* Re: [PATCH 5/6] bridge: new function to pack vlans using both IFLA_BRIDGE_VLAN_INFO and IFLA_BRIDGE_VLAN_RANGE_INFO
From: roopa @ 2014-12-30 13:57 UTC (permalink / raw)
To: Scott Feldman; +Cc: Netdev, shemminger, vyasevic@redhat.com, Wilson kok
In-Reply-To: <CAE4R7bAG2xdKDwXfFS=r4Vyk7hBF6dwRwfKVm0cSM2fTM0OHUw@mail.gmail.com>
On 12/30/14, 12:04 AM, Scott Feldman wrote:
> On Mon, Dec 29, 2014 at 9:31 PM, roopa <roopa@cumulusnetworks.com> wrote:
>> On 12/29/14, 3:25 PM, Scott Feldman wrote:
>>> On Mon, Dec 29, 2014 at 1:05 PM, <roopa@cumulusnetworks.com> wrote:
>>>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>>>
>>>> This patch adds new function to compress vlans into ranges.
>>>> Vlans are compressed into ranges only if the fill request is called with
>>>> RTEXT_FILTER_BRVLAN_COMPRESSED in filtermask.
>>>>
>>>> Old vlan packing code is moved to a new function and continues to be
>>>> called when filter_mask is RTEXT_FILTER_BRVLAN
>>>>
>>>> Signed-off-by: Wilson kok <wkok@cumulusnetworks.com>
>>>> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
>>>> ---
>>>> net/bridge/br_netlink.c | 157
>>>> +++++++++++++++++++++++++++++++++++++++++------
>>>> 1 file changed, 137 insertions(+), 20 deletions(-)
>>>>
>>>> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
>>>> index 4c47ba0..16bdd5a 100644
>>>> --- a/net/bridge/br_netlink.c
>>>> +++ b/net/bridge/br_netlink.c
>>>> @@ -67,6 +67,133 @@ static int br_port_fill_attrs(struct sk_buff *skb,
>>>> return 0;
>>>> }
>>>>
>>>> +static int br_fill_ifvlaninfo_bitmap(struct sk_buff *skb,
>>>> + const unsigned long *vlan_bmp, u16
>>>> flags)
>>>> +{
>>>> + struct bridge_vlan_range_info vinfo_range;
>>>> + struct bridge_vlan_info vinfo;
>>>> + u16 vid, start = 0, end = 0;
>>> One per line?
>> ack
>>
>>>> + u16 pvid;
>>> Aren't you getting an "unused" compile warning on this ^^^?
>> will double check..
>>
>>>> +
>>>> + /* handle the untagged */
>>> This comment ^^^ doesn't make sense here.
>>>
>>>> + for_each_set_bit(vid, vlan_bmp, VLAN_N_VID) {
>>>> + if (start == 0) {
>>>> + start = vid;
>>>> + end = vid;
>>>> + }
>>>> + if ((vid - end) > 1) {
>>>> + memset(&vinfo_range, 0, sizeof(vinfo_range));
>>>> + vinfo_range.flags |= flags;
>>>> + vinfo_range.vid = start;
>>>> + vinfo_range.vid_end = end;
>>>> + if (nla_put(skb, IFLA_BRIDGE_VLAN_RANGE_INFO,
>>>> + sizeof(vinfo_range), &vinfo_range))
>>>> + goto nla_put_failure;
>>>> + start = vid;
>>>> + end = vid;
>>>> + } else {
>>>> + end = vid;
>>>> + }
>>>> + }
>>> What happens with this set {1-10, 12, 20-25, 30}? On vid 12, will
>>> that put a VLAN_RANGE_INFO with start=end=12? Seems strange to use
>>> RANGE_INFO for single vlan.
>>>
>>> Can the algorithm be simplified? Maybe there are other examples in
>>> the kernel of finding ranges/singles from a bitmap we could borrow
>>> code from?
>> let me see...
>>
>>>> + if (start != 0 && end != 0) {
>>>> + if (start != end) {
>>>> + memset(&vinfo_range, 0, sizeof(vinfo_range));
>>>> + vinfo_range.flags |= flags;
>>>> + vinfo_range.vid = start;
>>>> + vinfo_range.vid_end = end;
>>>> + if (nla_put(skb, IFLA_BRIDGE_VLAN_RANGE_INFO,
>>>> + sizeof(vinfo_range), &vinfo_range))
>>>> + goto nla_put_failure;
>>>> + } else {
>>>> + memset(&vinfo, 0, sizeof(vinfo));
>>>> + vinfo.flags |= flags;
>>>> + vinfo.vid = start;
>>>> + if (nla_put(skb, IFLA_BRIDGE_VLAN_INFO,
>>>> + sizeof(vinfo), &vinfo))
>>>> + goto nla_put_failure;
>>> How many IFLA_BRIDGE_VLAN_INFOs can you fit into skb? It seems the
>>> worst case is you'll have 4094/2 = 2047 VLAN_INFOs if using all
>>> even-numbered vids, for example. Will that fit? I guess the original
>>> code had the same concern...wonder if anyone checked this worst-case?
>>
>> I will check this again. Remember doing worst case tests for setlinks. will
>> get back with some worst cases tests in the next submission.
>>>
>>>> + }
>>>> + }
>>>> +
>>>> +nla_put_failure:
>>>> + return -EMSGSIZE;
>>>> +}
>>>> +
>>>> +static int br_fill_ifvlaninfo_compressed(struct sk_buff *skb,
>>>> + const struct net_port_vlans *pv)
>>>> +{
>>>> + unsigned long vlan_bmp_copy[BR_VLAN_BITMAP_LEN];
>>>> + unsigned long untagged_bmp_copy[BR_VLAN_BITMAP_LEN];
>>> Lots of automatic space on the stack...can you use dynamic mem
>>> (kzalloc) for these?
>> Let me see. Or make it a static global ?
> Static global would be bad news for multiple threads doing a vlan fill.
They are all under rtnl_lock today. In any case, I do plan to change it
to kzalloc in its current place.
^ permalink raw reply
* [PATCH 4/4] ath10k: fixup wait_for_completion_timeout return handling
From: Nicholas Mc Guire @ 2014-12-30 12:20 UTC (permalink / raw)
To: Kalle Valo
Cc: Michal Kazior, Ben Greear, Chun-Yeow Yeoh, Yanbo Li, ath10k,
linux-wireless, netdev, linux-kernel, Nicholas Mc Guire
In-Reply-To: <1419942046-17985-1-git-send-email-der.herr@hofr.at>
wait_for_completion_timeout does not return negative values so
"ret" handling here should check for timeout only.
patch was only compile tested for x86_64_defconfig + CONFIG_ATH10K=m
patch is against linux-next 3.19.0-rc1 -next-20141226
Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at>
---
drivers/net/wireless/ath/ath10k/htt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ath/ath10k/htt.c b/drivers/net/wireless/ath/ath10k/htt.c
index 56cb4ac..58b6fc1 100644
--- a/drivers/net/wireless/ath/ath10k/htt.c
+++ b/drivers/net/wireless/ath/ath10k/htt.c
@@ -102,7 +102,7 @@ int ath10k_htt_setup(struct ath10k_htt *htt)
status = wait_for_completion_timeout(&htt->target_version_received,
HTT_TARGET_VERSION_TIMEOUT_HZ);
- if (status <= 0) {
+ if (status == 0) {
ath10k_warn(ar, "htt version request timed out\n");
return -ETIMEDOUT;
}
--
1.7.10.4
^ permalink raw reply related
* [PATCH 3/4] ath10k: fixup wait_for_completion_timeout return handling
From: Nicholas Mc Guire @ 2014-12-30 12:20 UTC (permalink / raw)
To: Kalle Valo
Cc: Michal Kazior, Ben Greear, Chun-Yeow Yeoh, Yanbo Li, ath10k,
linux-wireless, netdev, linux-kernel, Nicholas Mc Guire
In-Reply-To: <1419942046-17985-1-git-send-email-der.herr@hofr.at>
wait_for_completion_timeout does not return negative values so
"ret" handling here should check for timeout only.
patch was only compile tested for x86_64_defconfig + CONFIG_ATH10K=m
patch is against linux-next 3.19.0-rc1 -next-20141226
Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at>
---
drivers/net/wireless/ath/ath10k/htc.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/htc.c b/drivers/net/wireless/ath/ath10k/htc.c
index f1946a6..2fd9e18 100644
--- a/drivers/net/wireless/ath/ath10k/htc.c
+++ b/drivers/net/wireless/ath/ath10k/htc.c
@@ -703,11 +703,9 @@ int ath10k_htc_connect_service(struct ath10k_htc *htc,
/* wait for response */
status = wait_for_completion_timeout(&htc->ctl_resp,
ATH10K_HTC_CONN_SVC_TIMEOUT_HZ);
- if (status <= 0) {
- if (status == 0)
- status = -ETIMEDOUT;
+ if (status == 0) {
ath10k_err(ar, "Service connect timeout: %d\n", status);
- return status;
+ return -ETIMEDOUT;
}
/* we controlled the buffer creation, it's aligned */
--
1.7.10.4
^ permalink raw reply related
* [PATCH 2/4] ath10k: fixup wait_for_completion_timeout return handling
From: Nicholas Mc Guire @ 2014-12-30 12:20 UTC (permalink / raw)
To: Kalle Valo
Cc: Michal Kazior, Ben Greear, Chun-Yeow Yeoh, Yanbo Li, ath10k,
linux-wireless, netdev, linux-kernel, Nicholas Mc Guire
In-Reply-To: <1419942046-17985-1-git-send-email-der.herr@hofr.at>
wait_for_completion_timeout does not return negative values so
"ret" handling here should check for timeout only.
patch was only compile tested for x86_64_defconfig + CONFIG_ATH10K=m
patch is against linux-next 3.19.0-rc1 -next-20141226
Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at>
---
drivers/net/wireless/ath/ath10k/mac.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index c400567..f9d7dbb 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -2151,7 +2151,7 @@ void ath10k_offchan_tx_work(struct work_struct *work)
ret = wait_for_completion_timeout(&ar->offchan_tx_completed,
3 * HZ);
- if (ret <= 0)
+ if (ret == 0)
ath10k_warn(ar, "timed out waiting for offchannel skb %p\n",
skb);
--
1.7.10.4
^ permalink raw reply related
* [PATCH 1/4] ath10k: fixup wait_for_completion_timeout return handling
From: Nicholas Mc Guire @ 2014-12-30 12:20 UTC (permalink / raw)
To: Kalle Valo
Cc: Chun-Yeow Yeoh, netdev, linux-wireless, linux-kernel, ath10k,
Nicholas Mc Guire, Michal Kazior, Yanbo Li, Ben Greear
In-Reply-To: <1419942046-17985-1-git-send-email-der.herr@hofr.at>
wait_for_completion_timeout does not return negative values so
"ret" handling here should check for timeout only.
patch was only compile tested for x86_64_defconfig + CONFIG_ATH_DEBUG=y
patch is against linux-next 3.19.0-rc1 -next-20141226
Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at>
---
drivers/net/wireless/ath/ath10k/debug.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c
index a716758..7e1fe93 100644
--- a/drivers/net/wireless/ath/ath10k/debug.c
+++ b/drivers/net/wireless/ath/ath10k/debug.c
@@ -373,7 +373,7 @@ static int ath10k_debug_fw_stats_request(struct ath10k *ar)
ret = wait_for_completion_timeout(&ar->debug.fw_stats_complete,
1*HZ);
- if (ret <= 0)
+ if (ret == 0)
return -ETIMEDOUT;
spin_lock_bh(&ar->data_lock);
--
1.7.10.4
^ permalink raw reply related
* [PATCH 0/4] ath10k: a few incorrect return handling fix-up
From: Nicholas Mc Guire @ 2014-12-30 12:20 UTC (permalink / raw)
To: Kalle Valo
Cc: Michal Kazior, Ben Greear, Chun-Yeow Yeoh, Yanbo Li, ath10k,
linux-wireless, netdev, linux-kernel, Nicholas Mc Guire
wait_for_completion_timeout does not return negative values so the tests
for <= 0 are not needed and the case differentiation in the error handling
path unnecessary.
patch was only compile tested x86_64_defconfig + CONFIG_ATH_CARDS=m
CONFIG_ATH10K=m
patch is against linux-next 3.19.0-rc1 -next-20141226
None of the proposed cleanups are critical.
All changes should only be removing unreachable cases.
^ permalink raw reply
* Lucr@tive
From: Mark @ 2014-12-30 19:14 UTC (permalink / raw)
To: netdev
Hi
Forget the l0tteRY
This is re@l Joy and a real ch@nce for a new life No crap No Blabla
http://www.lotusasiacasino.com?cid=20W02156&mid=2228620828
No more such information? Simply answer <NO>
^ permalink raw reply
* [PATCH 3/3][v2] net/fsl: remove hardcoded clock setting from xgmac_mdio
From: shh.xie @ 2014-12-30 8:28 UTC (permalink / raw)
To: netdev, davem; +Cc: Shaohui Xie
From: Shaohui Xie <Shaohui.Xie@freescale.com>
There is no need to set the clock speed in read/write which will be performed
unnecessarily for each mdio access. Init it during probe is enough.
Also, the hardcoded clock value is not a proper way for all SoCs.
Signed-off-by: Shaohui Xie <Shaohui.Xie@freescale.com>
---
changes in v2:
no change.
drivers/net/ethernet/freescale/xgmac_mdio.c | 14 --------------
1 file changed, 14 deletions(-)
diff --git a/drivers/net/ethernet/freescale/xgmac_mdio.c b/drivers/net/ethernet/freescale/xgmac_mdio.c
index 72e0b85..f8c3bc0 100644
--- a/drivers/net/ethernet/freescale/xgmac_mdio.c
+++ b/drivers/net/ethernet/freescale/xgmac_mdio.c
@@ -94,13 +94,6 @@ static int xgmac_mdio_write(struct mii_bus *bus, int phy_id, int regnum, u16 val
uint16_t dev_addr = regnum >> 16;
int ret;
- /* Setup the MII Mgmt clock speed */
- out_be32(®s->mdio_stat, MDIO_STAT_CLKDIV(100));
-
- ret = xgmac_wait_until_free(&bus->dev, regs);
- if (ret)
- return ret;
-
/* Set the port and dev addr */
out_be32(®s->mdio_ctl,
MDIO_CTL_PORT_ADDR(phy_id) | MDIO_CTL_DEV_ADDR(dev_addr));
@@ -135,13 +128,6 @@ static int xgmac_mdio_read(struct mii_bus *bus, int phy_id, int regnum)
uint16_t value;
int ret;
- /* Setup the MII Mgmt clock speed */
- out_be32(®s->mdio_stat, MDIO_STAT_CLKDIV(100));
-
- ret = xgmac_wait_until_free(&bus->dev, regs);
- if (ret)
- return ret;
-
/* Set the Port and Device Addrs */
mdio_ctl = MDIO_CTL_PORT_ADDR(phy_id) | MDIO_CTL_DEV_ADDR(dev_addr);
out_be32(®s->mdio_ctl, mdio_ctl);
--
1.8.4.1
^ permalink raw reply related
* [PATCH net 1/2] net/mlx4_core: Correcly update the mtt's offset in the MR re-reg flow
From: Or Gerlitz @ 2014-12-30 9:59 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Matan Barak, talal, Maor Gottlieb, Or Gerlitz
In-Reply-To: <1419933590-9718-1-git-send-email-ogerlitz@mellanox.com>
From: Maor Gottlieb <maorg@mellanox.com>
Previously, mlx4_mt_rereg_write filled the MPT's entity_size with the
old MTT's page shift, which could result in using an incorrect offset.
Fix the initialization to be after we calculate the new MTT offset.
In addition, assign mtt order to -1 after calling mlx4_mtt_cleanup. This
is necessary in order to mark the MTT as invalid and avoid freeing it later.
Fixes: e630664 ('mlx4_core: Add helper functions to support MR re-registration')
Signed-off-by: Maor Gottlieb <maorg@mellanox.com>
Signed-off-by: Matan Barak <matanb@mellanox.com>
Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx4/mr.c | 9 +++++----
1 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/mr.c b/drivers/net/ethernet/mellanox/mlx4/mr.c
index d6f5496..7094a9c 100644
--- a/drivers/net/ethernet/mellanox/mlx4/mr.c
+++ b/drivers/net/ethernet/mellanox/mlx4/mr.c
@@ -584,6 +584,7 @@ EXPORT_SYMBOL_GPL(mlx4_mr_free);
void mlx4_mr_rereg_mem_cleanup(struct mlx4_dev *dev, struct mlx4_mr *mr)
{
mlx4_mtt_cleanup(dev, &mr->mtt);
+ mr->mtt.order = -1;
}
EXPORT_SYMBOL_GPL(mlx4_mr_rereg_mem_cleanup);
@@ -593,14 +594,14 @@ int mlx4_mr_rereg_mem_write(struct mlx4_dev *dev, struct mlx4_mr *mr,
{
int err;
- mpt_entry->start = cpu_to_be64(iova);
- mpt_entry->length = cpu_to_be64(size);
- mpt_entry->entity_size = cpu_to_be32(page_shift);
-
err = mlx4_mtt_init(dev, npages, page_shift, &mr->mtt);
if (err)
return err;
+ mpt_entry->start = cpu_to_be64(mr->iova);
+ mpt_entry->length = cpu_to_be64(mr->size);
+ mpt_entry->entity_size = cpu_to_be32(mr->mtt.page_shift);
+
mpt_entry->pd_flags &= cpu_to_be32(MLX4_MPT_PD_MASK |
MLX4_MPT_PD_FLAG_EN_INV);
mpt_entry->flags &= cpu_to_be32(MLX4_MPT_FLAG_FREE |
--
1.7.1
^ permalink raw reply related
* [PATCH net 0/2] mlx4 driver fixes for 3.19-rc2
From: Or Gerlitz @ 2014-12-30 9:59 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Matan Barak, talal, Or Gerlitz
Hi Dave,
Please push Maor's patch to -stable >= 3.17
Jack's fixes error-flow issues introduced in 3.19-rc1, no need for -stable.
thanks and EOY happy-holidays,
Or.
Jack Morgenstein (1):
net/mlx4_core: Fix error flow in mlx4_init_hca()
Maor Gottlieb (1):
net/mlx4_core: Correcly update the mtt's offset in the MR re-reg flow
drivers/net/ethernet/mellanox/mlx4/main.c | 13 ++++---------
drivers/net/ethernet/mellanox/mlx4/mr.c | 9 +++++----
2 files changed, 9 insertions(+), 13 deletions(-)
^ permalink raw reply
* [PATCH net 2/2] net/mlx4_core: Fix error flow in mlx4_init_hca()
From: Or Gerlitz @ 2014-12-30 9:59 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Matan Barak, talal, Jack Morgenstein, Or Gerlitz
In-Reply-To: <1419933590-9718-1-git-send-email-ogerlitz@mellanox.com>
From: Jack Morgenstein <jackm@dev.mellanox.co.il>
We shouldn't call UNMAP_FA here, this is done in mlx4_load_one.
If mlx4_query_func fails, we need to invoke CLOSE_HCA for both
native and master.
Fixes: a0eacca948d2 ('net/mlx4_core: Refactor mlx4_load_one')
Signed-off-by: Jack Morgenstein <jackm@dev.mellanox.co.il>
Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx4/main.c | 13 ++++---------
1 files changed, 4 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c
index 943cbd4..03e9eb0 100644
--- a/drivers/net/ethernet/mellanox/mlx4/main.c
+++ b/drivers/net/ethernet/mellanox/mlx4/main.c
@@ -1829,7 +1829,7 @@ static int mlx4_init_hca(struct mlx4_dev *dev)
err = mlx4_dev_cap(dev, &dev_cap);
if (err) {
mlx4_err(dev, "QUERY_DEV_CAP command failed, aborting\n");
- goto err_stop_fw;
+ return err;
}
choose_steering_mode(dev, &dev_cap);
@@ -1860,7 +1860,7 @@ static int mlx4_init_hca(struct mlx4_dev *dev)
&init_hca);
if ((long long) icm_size < 0) {
err = icm_size;
- goto err_stop_fw;
+ return err;
}
dev->caps.max_fmr_maps = (1 << (32 - ilog2(dev->caps.num_mpts))) - 1;
@@ -1874,7 +1874,7 @@ static int mlx4_init_hca(struct mlx4_dev *dev)
err = mlx4_init_icm(dev, &dev_cap, &init_hca, icm_size);
if (err)
- goto err_stop_fw;
+ return err;
err = mlx4_INIT_HCA(dev, &init_hca);
if (err) {
@@ -1886,7 +1886,7 @@ static int mlx4_init_hca(struct mlx4_dev *dev)
err = mlx4_query_func(dev, &dev_cap);
if (err < 0) {
mlx4_err(dev, "QUERY_FUNC command failed, aborting.\n");
- goto err_stop_fw;
+ goto err_close;
} else if (err & MLX4_QUERY_FUNC_NUM_SYS_EQS) {
dev->caps.num_eqs = dev_cap.max_eqs;
dev->caps.reserved_eqs = dev_cap.reserved_eqs;
@@ -2006,11 +2006,6 @@ err_free_icm:
if (!mlx4_is_slave(dev))
mlx4_free_icms(dev);
-err_stop_fw:
- if (!mlx4_is_slave(dev)) {
- mlx4_UNMAP_FA(dev);
- mlx4_free_icm(dev, priv->fw.fw_icm, 0);
- }
return err;
}
--
1.7.1
^ permalink raw reply related
* Re: [scsi/net-next]Pull csiostor from net-next
From: hch @ 2014-12-30 9:36 UTC (permalink / raw)
To: Praveen Madhavan; +Cc: linux-scsi@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <24C222ADD7729F4FB5113B41FF0028A1BCC18E@nice.asicdesigners.com>
On Sun, Dec 28, 2014 at 03:13:14PM +0000, Praveen Madhavan wrote:
> > How much do you plan to send for the 3.20 window? I'd rather avoid
> > having to pull in the net-next tree and merge everything through Dave
> > if that seems feasible.
> I hv couple of patches fixes for 3.20 window. Can you please pull from linux-next tree ?
If it's really just a few fixes I'd prefer to just ACK them on the scsi
list and send them through the net tree to avoid a cross dependency.
^ permalink raw reply
* [PATCH 2/3][v2] net/fsl: remove irq assignment from xgmac_mdio
From: shh.xie @ 2014-12-30 8:28 UTC (permalink / raw)
To: netdev, davem; +Cc: Shaohui Xie
From: Shaohui Xie <Shaohui.Xie@freescale.com>
Which is wrong and not used, so no extra space needed by
mdiobus_alloc_size(), use mdiobus_alloc() instead.
Signed-off-by: Shaohui Xie <Shaohui.Xie@freescale.com>
---
changes in v2:
use mdiobus_alloc() instead of mdiobus_alloc_size(0).
drivers/net/ethernet/freescale/xgmac_mdio.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/freescale/xgmac_mdio.c b/drivers/net/ethernet/freescale/xgmac_mdio.c
index 90adba1..72e0b85 100644
--- a/drivers/net/ethernet/freescale/xgmac_mdio.c
+++ b/drivers/net/ethernet/freescale/xgmac_mdio.c
@@ -187,14 +187,13 @@ static int xgmac_mdio_probe(struct platform_device *pdev)
return ret;
}
- bus = mdiobus_alloc_size(PHY_MAX_ADDR * sizeof(int));
+ bus = mdiobus_alloc();
if (!bus)
return -ENOMEM;
bus->name = "Freescale XGMAC MDIO Bus";
bus->read = xgmac_mdio_read;
bus->write = xgmac_mdio_write;
- bus->irq = bus->priv;
bus->parent = &pdev->dev;
snprintf(bus->id, MII_BUS_ID_SIZE, "%llx", (unsigned long long)res.start);
--
1.8.4.1
^ permalink raw reply related
* [PATCH 1/3][v2] net/fsl: remove reset from xgmac_mdio
From: shh.xie @ 2014-12-30 8:27 UTC (permalink / raw)
To: netdev, davem; +Cc: Shaohui Xie
From: Shaohui Xie <Shaohui.Xie@freescale.com>
Since the reset is just clock setting, individual mdio reset is
not available.
Signed-off-by: Shaohui Xie <Shaohui.Xie@freescale.com>
---
changes in v2:
no change.
drivers/net/ethernet/freescale/xgmac_mdio.c | 19 -------------------
1 file changed, 19 deletions(-)
diff --git a/drivers/net/ethernet/freescale/xgmac_mdio.c b/drivers/net/ethernet/freescale/xgmac_mdio.c
index 6e7db66..90adba1 100644
--- a/drivers/net/ethernet/freescale/xgmac_mdio.c
+++ b/drivers/net/ethernet/freescale/xgmac_mdio.c
@@ -174,24 +174,6 @@ static int xgmac_mdio_read(struct mii_bus *bus, int phy_id, int regnum)
return value;
}
-/* Reset the MIIM registers, and wait for the bus to free */
-static int xgmac_mdio_reset(struct mii_bus *bus)
-{
- struct tgec_mdio_controller __iomem *regs = bus->priv;
- int ret;
-
- mutex_lock(&bus->mdio_lock);
-
- /* Setup the MII Mgmt clock speed */
- out_be32(®s->mdio_stat, MDIO_STAT_CLKDIV(100));
-
- ret = xgmac_wait_until_free(&bus->dev, regs);
-
- mutex_unlock(&bus->mdio_lock);
-
- return ret;
-}
-
static int xgmac_mdio_probe(struct platform_device *pdev)
{
struct device_node *np = pdev->dev.of_node;
@@ -212,7 +194,6 @@ static int xgmac_mdio_probe(struct platform_device *pdev)
bus->name = "Freescale XGMAC MDIO Bus";
bus->read = xgmac_mdio_read;
bus->write = xgmac_mdio_write;
- bus->reset = xgmac_mdio_reset;
bus->irq = bus->priv;
bus->parent = &pdev->dev;
snprintf(bus->id, MII_BUS_ID_SIZE, "%llx", (unsigned long long)res.start);
--
1.8.4.1
^ permalink raw reply related
* [Question] What's the noop_qdisc introduced for in the kernel?
From: Dennis Chen @ 2014-12-30 9:23 UTC (permalink / raw)
To: netdev
After google and the code reading, seems this Qdisc instance is only
used for the initialization purpose, I can't find the reason that this
object introduced in the kernel. Does anybody know what the historical
reason is for this invention? the purpose or the benefit for this
Qdisc object?
--
Den
^ permalink raw reply
* RE: [PATCH] qlcnic: Fix return value in qlcnic_probe()
From: Shahed Shaikh @ 2014-12-30 8:45 UTC (permalink / raw)
To: xuyongjiande@163.com, Dept-GE Linux NIC Dev
Cc: netdev, linux-kernel, Yongjian Xu
In-Reply-To: <1419926626-26624-1-git-send-email-xuyongjiande@163.com>
> -----Original Message-----
> From: xuyongjiande@163.com [mailto:xuyongjiande@163.com]
> Sent: Tuesday, December 30, 2014 1:34 PM
> To: Shahed Shaikh; Dept-GE Linux NIC Dev
> Cc: netdev; linux-kernel; Yongjian Xu
> Subject: [PATCH] qlcnic: Fix return value in qlcnic_probe()
>
> From: Yongjian Xu <xuyongjiande@gmail.com>
>
> If the check of adapter fails and goes into the 'else' branch, the return value
> 'err' should not still be zero.
>
> Signed-off-by: Yongjian Xu <xuyongjiande@gmail.com>
> ---
> drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c | 1 +
> 1 file changed, 1 insertion(+)
Acked-by: Shahed Shaikh <shahed.shaikh@qlogic.com>
Thanks,
Shahed
^ permalink raw reply
* RE: [PATCH 3/6] bridge: modify IFLA_AF_SPEC parser to parse IFLA_BRIDGE_VLAN_RANGE_INFO
From: Arad, Ronen @ 2014-12-30 8:40 UTC (permalink / raw)
To: roopa@cumulusnetworks.com, netdev@vger.kernel.org,
shemminger@vyatta.com, vyasevic@redhat.com
Cc: Wilson kok
In-Reply-To: <1419887132-7084-4-git-send-email-roopa@cumulusnetworks.com>
>-----Original Message-----
>From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On
>Behalf Of roopa@cumulusnetworks.com
>Sent: Monday, December 29, 2014 11:05 PM
>To: netdev@vger.kernel.org; shemminger@vyatta.com; vyasevic@redhat.com
>Cc: Roopa Prabhu; Wilson kok
>Subject: [PATCH 3/6] bridge: modify IFLA_AF_SPEC parser to parse
>IFLA_BRIDGE_VLAN_RANGE_INFO
>
>From: Roopa Prabhu <roopa@cumulusnetworks.com>
>
>This patch modifies br_afspec to parse incoming IFLA_BRIDGE_VLAN_RANGE_INFO
>
>Signed-off-by: Wilson kok <wkok@cumulusnetworks.com>
>Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
>---
> net/bridge/br_netlink.c | 70 +++++++++++++++++++++++++++++++++-------------
>-
> 1 file changed, 49 insertions(+), 21 deletions(-)
>
>diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
>index e7d1fc0..4c47ba0 100644
>--- a/net/bridge/br_netlink.c
>+++ b/net/bridge/br_netlink.c
>@@ -227,48 +227,76 @@ static const struct nla_policy
>ifla_br_policy[IFLA_MAX+1] = {
> .len = sizeof(struct bridge_vlan_range_info), },
> };
>
>+static int br_afspec_vlan_add(struct net_bridge *br,
>+ struct net_bridge_port *p,
>+ u16 vid, u16 flags)
>+{
>+ int err = 0;
>+
>+ if (p) {
>+ err = nbp_vlan_add(p, vid, flags);
>+ if (err)
>+ return err;
>+
>+ if (flags & BRIDGE_VLAN_INFO_MASTER)
>+ err = br_vlan_add(p->br, vid, flags);
>+ } else {
>+ err = br_vlan_add(br, vid, flags);
>+ }
>+
>+ return err;
>+}
>+
>+static void br_afspec_vlan_del(struct net_bridge *br,
>+ struct net_bridge_port *p,
>+ u16 vid, u16 flags)
>+{
>+ if (p) {
>+ nbp_vlan_delete(p, vid);
>+ if (flags & BRIDGE_VLAN_INFO_MASTER)
>+ br_vlan_delete(p->br, vid);
>+ } else {
>+ br_vlan_delete(br, vid);
>+ }
>+}
>+
> static int br_afspec(struct net_bridge *br,
> struct net_bridge_port *p,
> struct nlattr *af_spec,
> int cmd)
> {
>- struct bridge_vlan_info *vinfo;
>- int err = 0;
>+ struct bridge_vlan_range_info *vinfo;
> struct nlattr *attr;
> int err = 0;
> int rem;
> u16 vid;
>
> nla_for_each_nested(attr, af_spec, rem) {
>- if (nla_type(attr) != IFLA_BRIDGE_VLAN_INFO)
>+ if (nla_type(attr) != IFLA_BRIDGE_VLAN_INFO &&
>+ nla_type(attr) != IFLA_BRIDGE_VLAN_RANGE_INFO)
> continue;
>-
> vinfo = nla_data(attr);
>- if (!vinfo->vid || vinfo->vid >= VLAN_VID_MASK)
>+
>+ if (nla_type(attr) == IFLA_BRIDGE_VLAN_INFO)
>+ vinfo->vid_end = vinfo->vid;
>+
>+ if (!vinfo->vid || vinfo->vid >= VLAN_VID_MASK ||
>+ vinfo->vid_end >= VLAN_VID_MASK ||
>+ vinfo->vid > vinfo->vid_end)
> return -EINVAL;
>
> switch (cmd) {
> case RTM_SETLINK:
>- if (p) {
>- err = nbp_vlan_add(p, vinfo->vid, vinfo->flags);
>+ for (vid = vinfo->vid; vid <= vinfo->vid_end; vid++) {
>+ err = br_afspec_vlan_add(br, p, vid,
>+ vinfo->flags);
vinfo->flags could have BRIDGE_VLAN_INFO_PVID set. It is really a port property and there could only be a single PVID for a port. The loop will make the port pvid set, in turn, to each vid in the range until it finally set to vid_end.
This could be avoided by turning off this flag for all but the last vid in range:
err = br_afspec_vlan_addr(br, p, vid,
((vid == vinfo->vid_end)
? vinfo->flags
: vinfo->flags & ~BRIDGE_VLAN_INFO_PVID));
Another alternative could be to add explicit pvid field to bridge_vlan_range_info and disallow PVID flag.
This allows for setting the port pvid to any vid in the range.
If both alternatives seem somewhat complex we could just disallow PVID flag in IFLA_BRIDGE_VLAN_RANGE_INFO and allow it only in IFLA_BRIDGE_VLAN_INFO.
> if (err)
> break;
>-
>- if (vinfo->flags & BRIDGE_VLAN_INFO_MASTER)
>- err = br_vlan_add(p->br, vinfo->vid,
>- vinfo->flags);
>- } else
>- err = br_vlan_add(br, vinfo->vid, vinfo->flags);
>-
>+ }
> break;
>-
> case RTM_DELLINK:
>- if (p) {
>- nbp_vlan_delete(p, vinfo->vid);
>- if (vinfo->flags & BRIDGE_VLAN_INFO_MASTER)
>- br_vlan_delete(p->br, vinfo->vid);
>- } else
>- br_vlan_delete(br, vinfo->vid);
>+ for (vid = vinfo->vid; vid <= vinfo->vid_end; vid++)
>+ br_afspec_vlan_del(br, p, vid, vinfo->flags);
> break;
> }
> }
>--
>1.7.10.4
>
>--
>To unsubscribe from this list: send the line "unsubscribe netdev" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] bonding: move ipoib_header_ops to vmlinux
From: Wengang @ 2014-12-30 8:26 UTC (permalink / raw)
To: David Miller; +Cc: cwang, netdev, linux-rdma
In-Reply-To: <20141229.232507.2086816594770925202.davem@davemloft.net>
于 2014年12月30日 12:25, David Miller 写道:
> From: Wengang <wen.gang.wang@oracle.com>
> Date: Tue, 30 Dec 2014 11:01:42 +0800
>
>> There are more than one way we do things. For this case, considering
>> needs, complexity and stability I think moving ipoib_header_ops is the
>> right way to go.
> I completely disagree, it's a gross hack at best.
>
> It's papering over the real problem.
>
> When we have references to released objects in other areas of the
> networking stack, we don't move those objects into the static kernel
> image as a fix.
OK. Let me see if I can make a patch to match what you want.
Thanks
wengang
^ permalink raw reply
* Re: [PATCH 5/6] bridge: new function to pack vlans using both IFLA_BRIDGE_VLAN_INFO and IFLA_BRIDGE_VLAN_RANGE_INFO
From: Scott Feldman @ 2014-12-30 8:04 UTC (permalink / raw)
To: roopa; +Cc: Netdev, shemminger, vyasevic@redhat.com, Wilson kok
In-Reply-To: <54A238BD.7050707@cumulusnetworks.com>
On Mon, Dec 29, 2014 at 9:31 PM, roopa <roopa@cumulusnetworks.com> wrote:
> On 12/29/14, 3:25 PM, Scott Feldman wrote:
>>
>> On Mon, Dec 29, 2014 at 1:05 PM, <roopa@cumulusnetworks.com> wrote:
>>>
>>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>>
>>> This patch adds new function to compress vlans into ranges.
>>> Vlans are compressed into ranges only if the fill request is called with
>>> RTEXT_FILTER_BRVLAN_COMPRESSED in filtermask.
>>>
>>> Old vlan packing code is moved to a new function and continues to be
>>> called when filter_mask is RTEXT_FILTER_BRVLAN
>>>
>>> Signed-off-by: Wilson kok <wkok@cumulusnetworks.com>
>>> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
>>> ---
>>> net/bridge/br_netlink.c | 157
>>> +++++++++++++++++++++++++++++++++++++++++------
>>> 1 file changed, 137 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
>>> index 4c47ba0..16bdd5a 100644
>>> --- a/net/bridge/br_netlink.c
>>> +++ b/net/bridge/br_netlink.c
>>> @@ -67,6 +67,133 @@ static int br_port_fill_attrs(struct sk_buff *skb,
>>> return 0;
>>> }
>>>
>>> +static int br_fill_ifvlaninfo_bitmap(struct sk_buff *skb,
>>> + const unsigned long *vlan_bmp, u16
>>> flags)
>>> +{
>>> + struct bridge_vlan_range_info vinfo_range;
>>> + struct bridge_vlan_info vinfo;
>>> + u16 vid, start = 0, end = 0;
>>
>> One per line?
>
> ack
>
>>> + u16 pvid;
>>
>> Aren't you getting an "unused" compile warning on this ^^^?
>
> will double check..
>
>>> +
>>> + /* handle the untagged */
>>
>> This comment ^^^ doesn't make sense here.
>>
>>> + for_each_set_bit(vid, vlan_bmp, VLAN_N_VID) {
>>> + if (start == 0) {
>>> + start = vid;
>>> + end = vid;
>>> + }
>>> + if ((vid - end) > 1) {
>>> + memset(&vinfo_range, 0, sizeof(vinfo_range));
>>> + vinfo_range.flags |= flags;
>>> + vinfo_range.vid = start;
>>> + vinfo_range.vid_end = end;
>>> + if (nla_put(skb, IFLA_BRIDGE_VLAN_RANGE_INFO,
>>> + sizeof(vinfo_range), &vinfo_range))
>>> + goto nla_put_failure;
>>> + start = vid;
>>> + end = vid;
>>> + } else {
>>> + end = vid;
>>> + }
>>> + }
>>
>> What happens with this set {1-10, 12, 20-25, 30}? On vid 12, will
>> that put a VLAN_RANGE_INFO with start=end=12? Seems strange to use
>> RANGE_INFO for single vlan.
>>
>> Can the algorithm be simplified? Maybe there are other examples in
>> the kernel of finding ranges/singles from a bitmap we could borrow
>> code from?
>
> let me see...
>
>>> + if (start != 0 && end != 0) {
>>> + if (start != end) {
>>> + memset(&vinfo_range, 0, sizeof(vinfo_range));
>>> + vinfo_range.flags |= flags;
>>> + vinfo_range.vid = start;
>>> + vinfo_range.vid_end = end;
>>> + if (nla_put(skb, IFLA_BRIDGE_VLAN_RANGE_INFO,
>>> + sizeof(vinfo_range), &vinfo_range))
>>> + goto nla_put_failure;
>>> + } else {
>>> + memset(&vinfo, 0, sizeof(vinfo));
>>> + vinfo.flags |= flags;
>>> + vinfo.vid = start;
>>> + if (nla_put(skb, IFLA_BRIDGE_VLAN_INFO,
>>> + sizeof(vinfo), &vinfo))
>>> + goto nla_put_failure;
>>
>> How many IFLA_BRIDGE_VLAN_INFOs can you fit into skb? It seems the
>> worst case is you'll have 4094/2 = 2047 VLAN_INFOs if using all
>> even-numbered vids, for example. Will that fit? I guess the original
>> code had the same concern...wonder if anyone checked this worst-case?
>
>
> I will check this again. Remember doing worst case tests for setlinks. will
> get back with some worst cases tests in the next submission.
>>
>>
>>> + }
>>> + }
>>> +
>>> +nla_put_failure:
>>> + return -EMSGSIZE;
>>> +}
>>> +
>>> +static int br_fill_ifvlaninfo_compressed(struct sk_buff *skb,
>>> + const struct net_port_vlans *pv)
>>> +{
>>> + unsigned long vlan_bmp_copy[BR_VLAN_BITMAP_LEN];
>>> + unsigned long untagged_bmp_copy[BR_VLAN_BITMAP_LEN];
>>
>> Lots of automatic space on the stack...can you use dynamic mem
>> (kzalloc) for these?
>
> Let me see. Or make it a static global ?
Static global would be bad news for multiple threads doing a vlan fill.
^ permalink raw reply
* [PATCH] qlcnic: Fix return value in qlcnic_probe()
From: xuyongjiande @ 2014-12-30 8:03 UTC (permalink / raw)
To: shahed.shaikh, Dept-GELinuxNICDev; +Cc: netdev, linux-kernel, Yongjian Xu
From: Yongjian Xu <xuyongjiande@gmail.com>
If the check of adapter fails and goes into the 'else' branch, the
return value 'err' should not still be zero.
Signed-off-by: Yongjian Xu <xuyongjiande@gmail.com>
---
drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
index 1aa25b1..a95b604 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
@@ -2603,6 +2603,7 @@ qlcnic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
} else {
dev_err(&pdev->dev,
"%s: failed. Please Reboot\n", __func__);
+ err = -ENODEV;
goto err_out_free_hw;
}
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH 3/3] net: ieee802154: don't use devm_pinctrl_get_select_default() in probe
From: Marcel Holtmann @ 2014-12-30 6:17 UTC (permalink / raw)
To: Wolfram Sang
Cc: Linux Kernel Mailing List, linux-gpio, Varka Bhadram,
Alexander Aring, linux-wpan, netdev
In-Reply-To: <1419286616-9893-3-git-send-email-wsa@the-dreams.de>
Hi Wolfram,
> Since commit ab78029ecc34 (drivers/pinctrl: grab default handles from device
> core), we can rely on device core for setting the default pins.
>
> Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
> ---
> drivers/net/ieee802154/cc2520.c | 7 -------
> 1 file changed, 7 deletions(-)
patch has been applied to bluetooth-next tree.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 1/6] bridge: add support to parse multiple vlan info attributes in IFLA_AF_SPEC
From: roopa @ 2014-12-30 6:01 UTC (permalink / raw)
To: Scott Feldman; +Cc: Netdev, shemminger, vyasevic@redhat.com, Wilson Kok
In-Reply-To: <CAE4R7bDjmHrW+bzoPJ-fpN51ykTnqjrXy7hCqXgJNv-EHayiHg@mail.gmail.com>
On 12/29/14, 9:31 PM, Scott Feldman wrote:
> On Mon, Dec 29, 2014 at 9:25 PM, roopa <roopa@cumulusnetworks.com> wrote:
>> On 12/29/14, 4:26 PM, Scott Feldman wrote:
>>> On Mon, Dec 29, 2014 at 4:07 PM, Scott Feldman <sfeldma@gmail.com> wrote:
>>>> On Mon, Dec 29, 2014 at 3:47 PM, Scott Feldman <sfeldma@gmail.com> wrote:
>>>>> On Mon, Dec 29, 2014 at 2:10 PM, roopa <roopa@cumulusnetworks.com>
>>>>> wrote:
>>>>>> On 12/29/14, 1:40 PM, Scott Feldman wrote:
>>>>>>> On Mon, Dec 29, 2014 at 1:05 PM, <roopa@cumulusnetworks.com> wrote:
>>>>>>>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>>>>>>>
>>>>>>>> This patch changes bridge IFLA_AF_SPEC netlink attribute parser to
>>>>>>>> look for more than one IFLA_BRIDGE_VLAN_INFO attribute. This allows
>>>>>>>> userspace to pack more than one vlan in the setlink msg.
>>>>>>>>
>>>>>>>> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
>>>>>>>> ---
>>>>>>>> net/bridge/br_netlink.c | 18 +++++++++---------
>>>>>>>> 1 file changed, 9 insertions(+), 9 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
>>>>>>>> index 9f5eb55..75971b1 100644
>>>>>>>> --- a/net/bridge/br_netlink.c
>>>>>>>> +++ b/net/bridge/br_netlink.c
>>>>>>>> @@ -230,18 +230,18 @@ static int br_afspec(struct net_bridge *br,
>>>>>>>> struct nlattr *af_spec,
>>>>>>>> int cmd)
>>>>>>>> {
>>>>>>>> - struct nlattr *tb[IFLA_BRIDGE_MAX+1];
>>>>>>>> + struct bridge_vlan_info *vinfo;
>>>>>>>> int err = 0;
>>>>>>>> + struct nlattr *attr;
>>>>>>>> + int err = 0;
>>>>>>>> + int rem;
>>>>>>>> + u16 vid;
>>>>>>>>
>>>>>>>> - err = nla_parse_nested(tb, IFLA_BRIDGE_MAX, af_spec,
>>>>>>>> ifla_br_policy);
>>>>>>> Removing this call orphans ifla_br_policy...should ifla_br_policy be
>>>>>>> removed?
>>>>>>
>>>>>> good question. Its a good place to see the type. In-fact userspace
>>>>>> programs
>>>>>> also copy the same policy to parse netlink attributes. hmmm..
>>>>>> I would like to keep it if it does not throw a warning.
>>>>> I don't know what the policy (sorry, no pun intended) on leaving dead
>>>>> code. I say remove it.
>>>> You know, not using the policy seems like a step backwards, and maybe
>>>> it suggests a problem with the attr packing.
>>>>
>>>> We had:
>>>>
>>>> ifla_br_policy
>>>> IFLA_BRIDGE_FLAGS
>>>> IFLA_BRIDGE_MODE
>>>> IFLA_BRIDGE_VLAN_INFO
>>>>
>>>> This patch set makes it:
>>>>
>>>> ifla_br_policy
>>>> IFLA_BRIDGE_FLAGS
>>>> IFLA_BRIDGE_MODE
>>>> IFLA_BRIDGE_VLAN_INFO
>>>> IFLA_BRIDGE_VLAN_RANGE_INFO
>>>>
>>>> Which is fine, but now VLAN_INFO and VLAN_RANGE_INFO can be repeated.
>>>> I think you want some nesting to clarify:
>>>>
>>>> ifla_br_policy
>>>> IFLA_BRIDGE_FLAGS
>>>> IFLA_BRIDGE_MODE
>>>> IFLA_BRIDGE_VLAN_INFO
>>>> IFLA_BRIDGE_VLAN_LIST_INFO // nested array of
>>>> IFLA_BRIDGE_VLAN_INFO
>>>> IFLA_BRIDGE_VLAN_RANGE_INFO
>>>>
>>>> Now you can keep the policy for the top-level parsing, and loop only
>>>> on the nested array VLAN_LIST_INFO. Actually, now you can use just
>>>> RANGE_INFO in array and have:
>>>>
>>>> ifla_br_policy
>>>> IFLA_BRIDGE_FLAGS
>>>> IFLA_BRIDGE_MODE
>>>> IFLA_BRIDGE_VLAN_INFO
>>>> IFLA_BRIDGE_VLAN_LIST_INFO // nested array of
>>>> IFLA_BRIDGE_VLAN_RANGE_INFO
>>>>
>>>> And use VLAN_RANGE_INFO for both ranges of vids as well as single
>>>> vids. That'll simplify your filling algo in patch 5.
>>> Hmmmm...do you even need VLAN_RANGE_INFO? How about just using
>>> existing VLAN_INFO and add some more flags:
>>>
>>> #define BRIDGE_VLAN_INFO_RANGE_START (1<<3)
>>> #define BRIDGE_VLAN_INFO_RANGE_END (1<<4)
>>>
>>> Now you can have:
>>>
>>> IFLA_BRIDGE_FLAGS
>>> IFLA_BRIDGE_MODE
>>> IFLA_BRIDGE_VLAN_INFO
>>> IFLA_BRIDGE_VLAN_INFO_LIST // nested array of
>>> IFLA_BRIDGE_VLAN_INFO
>>>
>>> Don't set START or END for single vids in list.
>>
>> ok. I was debating yesterday about introducing another nest. This looks
>> good.
>> My only reason to not use existing IFLA_BRIDGE_VLAN_INFO was to make sure it
>> works for existing users.
>> I see that in this case since IFLA_BRIDGE_VLAN_INFO_LIST is new, it will not
>> affect existing users.
>>
>> But, i cant use IFLA_BRIDGE_VLAN_INFO (ie an attribute in ifla_br_policy)
>> under IFLA_BRIDGE_VLAN_INFO_LIST ?.
> I don't see why not. You're not going to parse the array with a
> policy anyway (since it's an array). And attr INFO_LIST will be .type
> = NLA_NESTED in ifla_br_policy.
>
>
agree that it will work if everybody assumes that this is an array of
just this attrtype.
wasn't sure if it is a good practice to reuse an attribute from an upper
nest.
will work on v2. thanks for the review.
^ permalink raw reply
* Re: [PATCH 6/6] bridge: add vlan info to bridge setlink and dellink notification messages
From: roopa @ 2014-12-30 5:41 UTC (permalink / raw)
To: Scott Feldman; +Cc: Netdev, shemminger, vyasevic@redhat.com, Wilson Kok
In-Reply-To: <CAE4R7bC406Vn_vCnbmoDNGMhNZTcO_sUu5CdOU7iefVOBZ-z6A@mail.gmail.com>
On 12/29/14, 9:25 PM, Scott Feldman wrote:
> On Mon, Dec 29, 2014 at 9:17 PM, roopa <roopa@cumulusnetworks.com> wrote:
>> On 12/29/14, 3:42 PM, Scott Feldman wrote:
>>> On Mon, Dec 29, 2014 at 1:05 PM, <roopa@cumulusnetworks.com> wrote:
>>>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>>>
>>>> vlan add/deletes are not notified to userspace today. This patch fixes
>>>> it.
>>>> Notifications will contain vlans compressed into ranges whereever
>>>> applicable.
>>> Why is this notification needed? On VLAN add/del, the port driver
>>> will already get called if it implements ndo_vlan_rx_add_vid and
>>> ndo_vlan_rx_kill_vid.
>> This is strictly for userspace.
>>
>>> The port driver already gets the notification
>>> it needs to filter vlans. User space can query for vlan port
>>> membership if it needs to know.
>>
>> You mean request a dump ?. yes it can if it needs all the configured vlans.
>> But for notifications, we must at the least notify what changed with respect
>> to vlans with RTM_SETLINK/DELLINK, no ?
> You'll get that notification if you implement ndo_vlan_rx_add/kill_vid
> in your port driver, which is called from RTM_SETLINK/DELLINK. Your
> port driver can send to user-space, I suppose, if user-space needs
> notification.
When the bridge is a vlan filtering bridge, I think its the bridge
driver who will need to send notifications to vlan add/dels
on PF_BRIDGE similar to fdb add/dels.
^ permalink raw reply
* Re: [PATCH 1/6] bridge: add support to parse multiple vlan info attributes in IFLA_AF_SPEC
From: Scott Feldman @ 2014-12-30 5:31 UTC (permalink / raw)
To: roopa; +Cc: Netdev, shemminger, vyasevic@redhat.com, Wilson Kok
In-Reply-To: <54A2374F.6050901@cumulusnetworks.com>
On Mon, Dec 29, 2014 at 9:25 PM, roopa <roopa@cumulusnetworks.com> wrote:
> On 12/29/14, 4:26 PM, Scott Feldman wrote:
>>
>> On Mon, Dec 29, 2014 at 4:07 PM, Scott Feldman <sfeldma@gmail.com> wrote:
>>>
>>> On Mon, Dec 29, 2014 at 3:47 PM, Scott Feldman <sfeldma@gmail.com> wrote:
>>>>
>>>> On Mon, Dec 29, 2014 at 2:10 PM, roopa <roopa@cumulusnetworks.com>
>>>> wrote:
>>>>>
>>>>> On 12/29/14, 1:40 PM, Scott Feldman wrote:
>>>>>>
>>>>>> On Mon, Dec 29, 2014 at 1:05 PM, <roopa@cumulusnetworks.com> wrote:
>>>>>>>
>>>>>>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>>>>>>
>>>>>>> This patch changes bridge IFLA_AF_SPEC netlink attribute parser to
>>>>>>> look for more than one IFLA_BRIDGE_VLAN_INFO attribute. This allows
>>>>>>> userspace to pack more than one vlan in the setlink msg.
>>>>>>>
>>>>>>> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
>>>>>>> ---
>>>>>>> net/bridge/br_netlink.c | 18 +++++++++---------
>>>>>>> 1 file changed, 9 insertions(+), 9 deletions(-)
>>>>>>>
>>>>>>> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
>>>>>>> index 9f5eb55..75971b1 100644
>>>>>>> --- a/net/bridge/br_netlink.c
>>>>>>> +++ b/net/bridge/br_netlink.c
>>>>>>> @@ -230,18 +230,18 @@ static int br_afspec(struct net_bridge *br,
>>>>>>> struct nlattr *af_spec,
>>>>>>> int cmd)
>>>>>>> {
>>>>>>> - struct nlattr *tb[IFLA_BRIDGE_MAX+1];
>>>>>>> + struct bridge_vlan_info *vinfo;
>>>>>>> int err = 0;
>>>>>>> + struct nlattr *attr;
>>>>>>> + int err = 0;
>>>>>>> + int rem;
>>>>>>> + u16 vid;
>>>>>>>
>>>>>>> - err = nla_parse_nested(tb, IFLA_BRIDGE_MAX, af_spec,
>>>>>>> ifla_br_policy);
>>>>>>
>>>>>> Removing this call orphans ifla_br_policy...should ifla_br_policy be
>>>>>> removed?
>>>>>
>>>>>
>>>>> good question. Its a good place to see the type. In-fact userspace
>>>>> programs
>>>>> also copy the same policy to parse netlink attributes. hmmm..
>>>>> I would like to keep it if it does not throw a warning.
>>>>
>>>> I don't know what the policy (sorry, no pun intended) on leaving dead
>>>> code. I say remove it.
>>>
>>> You know, not using the policy seems like a step backwards, and maybe
>>> it suggests a problem with the attr packing.
>>>
>>> We had:
>>>
>>> ifla_br_policy
>>> IFLA_BRIDGE_FLAGS
>>> IFLA_BRIDGE_MODE
>>> IFLA_BRIDGE_VLAN_INFO
>>>
>>> This patch set makes it:
>>>
>>> ifla_br_policy
>>> IFLA_BRIDGE_FLAGS
>>> IFLA_BRIDGE_MODE
>>> IFLA_BRIDGE_VLAN_INFO
>>> IFLA_BRIDGE_VLAN_RANGE_INFO
>>>
>>> Which is fine, but now VLAN_INFO and VLAN_RANGE_INFO can be repeated.
>>> I think you want some nesting to clarify:
>>>
>>> ifla_br_policy
>>> IFLA_BRIDGE_FLAGS
>>> IFLA_BRIDGE_MODE
>>> IFLA_BRIDGE_VLAN_INFO
>>> IFLA_BRIDGE_VLAN_LIST_INFO // nested array of
>>> IFLA_BRIDGE_VLAN_INFO
>>> IFLA_BRIDGE_VLAN_RANGE_INFO
>>>
>>> Now you can keep the policy for the top-level parsing, and loop only
>>> on the nested array VLAN_LIST_INFO. Actually, now you can use just
>>> RANGE_INFO in array and have:
>>>
>>> ifla_br_policy
>>> IFLA_BRIDGE_FLAGS
>>> IFLA_BRIDGE_MODE
>>> IFLA_BRIDGE_VLAN_INFO
>>> IFLA_BRIDGE_VLAN_LIST_INFO // nested array of
>>> IFLA_BRIDGE_VLAN_RANGE_INFO
>>>
>>> And use VLAN_RANGE_INFO for both ranges of vids as well as single
>>> vids. That'll simplify your filling algo in patch 5.
>>
>> Hmmmm...do you even need VLAN_RANGE_INFO? How about just using
>> existing VLAN_INFO and add some more flags:
>>
>> #define BRIDGE_VLAN_INFO_RANGE_START (1<<3)
>> #define BRIDGE_VLAN_INFO_RANGE_END (1<<4)
>>
>> Now you can have:
>>
>> IFLA_BRIDGE_FLAGS
>> IFLA_BRIDGE_MODE
>> IFLA_BRIDGE_VLAN_INFO
>> IFLA_BRIDGE_VLAN_INFO_LIST // nested array of
>> IFLA_BRIDGE_VLAN_INFO
>>
>> Don't set START or END for single vids in list.
>
>
> ok. I was debating yesterday about introducing another nest. This looks
> good.
> My only reason to not use existing IFLA_BRIDGE_VLAN_INFO was to make sure it
> works for existing users.
> I see that in this case since IFLA_BRIDGE_VLAN_INFO_LIST is new, it will not
> affect existing users.
>
> But, i cant use IFLA_BRIDGE_VLAN_INFO (ie an attribute in ifla_br_policy)
> under IFLA_BRIDGE_VLAN_INFO_LIST ?.
I don't see why not. You're not going to parse the array with a
policy anyway (since it's an array). And attr INFO_LIST will be .type
= NLA_NESTED in ifla_br_policy.
>
> IFLA_BRIDGE_VLAN_INFO_LIST will have its own policy and its own attributes.
>
> Which will make it look something like below ?
>
> IFLA_BRIDGE_FLAGS
> IFLA_BRIDGE_MODE
> IFLA_BRIDGE_VLAN_INFO
> IFLA_BRIDGE_VLAN_INFO_LIST // nested array of
> IFLA_BRIDGE_VLAN_INFO_LIST_ENTRY
>
>
> Thanks,
> Roopa
^ 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