* [PATCH] net:ethernet:intel:Remove outdated fix me comment in the function, gb_acquire_swfw_sync_i210
From: Nicholas Krause @ 2015-02-08 5:21 UTC (permalink / raw)
To: jeffrey.t.kirsher
Cc: linux.nics, e1000-devel, bruce.w.allan, jesse.brandeburg,
linux-kernel, john.ronciak, netdev
Removes the outdated fix me comment in the function,gb_acquire_swfw_sync_i210
for setting the variables, i and timeout to the intended correct values for
the function,gb_acquire_swfw_sync_i210 to function correctly. This comment is
no longer due to these values having been no changed in the last few years and
no known issues have been found for these variables being set to their current
values. Due to this the comment can now be removed as the values set for these
variables are known to be correct after years of no known issues or bugs related
to their current values.
Signed-off-by: Nicholas Krause <xerofoify@gmail.com>
---
drivers/net/ethernet/intel/igb/e1000_i210.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/igb/e1000_i210.c b/drivers/net/ethernet/intel/igb/e1000_i210.c
index 65d9316..0c169e5 100644
--- a/drivers/net/ethernet/intel/igb/e1000_i210.c
+++ b/drivers/net/ethernet/intel/igb/e1000_i210.c
@@ -140,7 +140,7 @@ s32 igb_acquire_swfw_sync_i210(struct e1000_hw *hw, u16 mask)
u32 swmask = mask;
u32 fwmask = mask << 16;
s32 ret_val = 0;
- s32 i = 0, timeout = 200; /* FIXME: find real value to use here */
+ s32 i = 0, timeout = 200;
while (i < timeout) {
if (igb_get_hw_semaphore_i210(hw)) {
--
2.1.0
------------------------------------------------------------------------------
Dive into the World of Parallel Programming. The Go Parallel Website,
sponsored by Intel and developed in partnership with Slashdot Media, is your
hub for all things parallel software development, from weekly thought
leadership blogs to news, videos, case studies, tutorials and more. Take a
look and join the conversation now. http://goparallel.sourceforge.net/
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply related
* Re: [PATCH next 2/6] bonding: implement bond_poll_controller()
From: Mahesh Bandewar @ 2015-02-08 4:19 UTC (permalink / raw)
To: Veaceslav Falico
Cc: Jay Vosburgh, Andy Gospodarek, Nikolay Aleksandrov, David Miller,
netdev, Eric Dumazet
In-Reply-To: <20150207090400.GA11343@vps.falico.eu>
On Sat, Feb 7, 2015 at 1:04 AM, Veaceslav Falico <vfalico@gmail.com> wrote:
> On Fri, Feb 06, 2015 at 04:51:51PM -0800, Mahesh Bandewar wrote:
>>
>> This patches implements the poll_controller support for all
>> bonding driver. If the slaves have poll_controller net_op defined,
>> this implementation calls them. This is mode agnostic implementation
>> and iterates through all slaves (based on mode) and calls respective
>> handler.
>>
>> Signed-off-by: Mahesh Bandewar <maheshb@google.com>
>
>
> Really good patchset, thank you. Two nitpicks below, nothing serious. Also,
> please, add Doc/bonding.txt changes for every user-visible
> change/enhancement, this doc is a go-to for a lot of users.
>
> Otherwise, with the doc and other issues, pointed by Jay and Andy, fixed, I
> guess it's good to go. The 0/ patch would be welcome too, btw.
>
>
>> ---
>> drivers/net/bonding/bond_3ad.c | 24 +++++++++++++++++++++
>> drivers/net/bonding/bond_main.c | 47
>> +++++++++++++++++++++++++++++++++++++++++
>> include/net/bond_3ad.h | 1 +
>> 3 files changed, 72 insertions(+)
>>
>> diff --git a/drivers/net/bonding/bond_3ad.c
>> b/drivers/net/bonding/bond_3ad.c
>> index 9b436696b95e..14f2ebe786c5 100644
>> --- a/drivers/net/bonding/bond_3ad.c
>> +++ b/drivers/net/bonding/bond_3ad.c
>> @@ -2477,6 +2477,30 @@ int bond_3ad_get_active_agg_info(struct bonding
>> *bond, struct ad_info *ad_info)
>> return ret;
>> }
>>
>> +#define BOND_3AD_PORT_OPERATIONAL \
>> + (AD_STATE_DISTRIBUTING | AD_STATE_COLLECTING | \
>> + AD_STATE_SYNCHRONIZATION | AD_STATE_AGGREGATION)
>> +
>> +static int bond_3ad_port_operational(struct slave *slave)
>> +{
>> + port_t *port = &SLAVE_AD_INFO(slave)->port;
>> +
>> + return bond_slave_can_tx(slave) &&
>> + (port->actor_oper_port_state &
>> port->partner_oper.port_state &
>> + BOND_3AD_PORT_OPERATIONAL) == BOND_3AD_PORT_OPERATIONAL;
>> +}
>> +
>> +/* bond_3ad_port_is_active - check if a slave port is active or not. A
>> port
>> + * is active when it can forward traffic.
>> + *
>> + * @slave: slave port to check state for.
>> + * Returns: 0 if not active else is active.
>> + */
>> +int bond_3ad_port_is_active(struct slave *slave)
>> +{
>> + return bond_3ad_port_operational(slave);
>> +}
>> +
>> int bond_3ad_lacpdu_recv(const struct sk_buff *skb, struct bonding *bond,
>> struct slave *slave)
>> {
>> diff --git a/drivers/net/bonding/bond_main.c
>> b/drivers/net/bonding/bond_main.c
>> index c9e519cb9214..a50ec87486f3 100644
>> --- a/drivers/net/bonding/bond_main.c
>> +++ b/drivers/net/bonding/bond_main.c
>> @@ -928,6 +928,53 @@ static inline void slave_disable_netpoll(struct slave
>> *slave)
>>
>> static void bond_poll_controller(struct net_device *bond_dev)
>> {
>> + struct bonding *bond = netdev_priv(bond_dev);
>> + struct slave *slave = NULL;
>> + struct list_head *iter;
>> + struct ad_info ad_info;
>> + struct aggregator *agg;
>> + const struct net_device_ops *ops;
>> + bool call_slave_netpoll;
>> +
>> + if (BOND_MODE(bond) == BOND_MODE_8023AD)
>> + if (bond_3ad_get_active_agg_info(bond, &ad_info))
>> + return;
>> +
>> + bond_for_each_slave(bond, slave, iter) {
>> + call_slave_netpoll = false;
>> + switch (BOND_MODE(bond)) {
>> + case BOND_MODE_8023AD:
>> + agg = SLAVE_AD_INFO(slave)->port.aggregator;
>> + if (!bond_slave_is_up(slave))
>
>
> bond_3ad_port_is_active() already contains the check for being up.
>
>> + break;
>> + if (agg && agg->aggregator_identifier !=
>
>
> Hm, should we poll it without an aggregator?
>
>> + ad_info.aggregator_id)
>> + break;
>> + if (!bond_3ad_port_is_active(slave) &&
>> + ad_info.ports != 1)
>> + break;
>> +
>> + call_slave_netpoll = true;
>> + break;
>> + default:
>> + if (bond_slave_is_up(slave))
>> + call_slave_netpoll = true;
>> + break;
>> + }
>> +
>> + if (call_slave_netpoll) {
>> + ops = slave->dev->netdev_ops;
>> + if (ops->ndo_poll_controller) {
>
>
> It's weird to see this check so down the road. Maybe reorg the logic to
> something like:
>
> bond_for_each_slave() {
> if (!bond_slave_is_up() || !bond_slave_has_ndo_poll())
> continue;
>
> if (BOND_MODE(bond) == 3AD && !bond_3ad_port_is_active())
> continue;
>
> slave-do_ndo_poll_stuff();
> }
>
> Just as a thought, might be easier to read/shorter.
>
>
OK, I'll try that. thanks
>> + struct netpoll_info *ni =
>> +
>> rcu_dereference_bh(slave->dev->npinfo);
>> +
>> + if (down_trylock(&ni->dev_lock))
>> + continue;
>> + ops->ndo_poll_controller(slave->dev);
>> + up(&ni->dev_lock);
>> + }
>> + }
>> + }
>> }
>>
>> static void bond_netpoll_cleanup(struct net_device *bond_dev)
>> diff --git a/include/net/bond_3ad.h b/include/net/bond_3ad.h
>> index f04cdbb7848e..6c455c646d61 100644
>> --- a/include/net/bond_3ad.h
>> +++ b/include/net/bond_3ad.h
>> @@ -278,5 +278,6 @@ int bond_3ad_lacpdu_recv(const struct sk_buff *skb,
>> struct bonding *bond,
>> struct slave *slave);
>> int bond_3ad_set_carrier(struct bonding *bond);
>> void bond_3ad_update_lacp_rate(struct bonding *bond);
>> +int bond_3ad_port_is_active(struct slave *slave);
>> #endif /* _NET_BOND_3AD_H */
>>
>> --
>> 2.2.0.rc0.207.ga3a616c
>>
>
^ permalink raw reply
* Re: [PATCH next 5/6] bonding: Allow userspace to set macaddr on bonding-dev
From: Mahesh Bandewar @ 2015-02-08 3:32 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Jay Vosburgh, Andy Gospodarek, Veaceslav Falico,
Nikolay Aleksandrov, David Miller, netdev, Eric Dumazet
In-Reply-To: <20150207142652.708bd66e@urahara>
On Sat, Feb 7, 2015 at 2:26 PM, Stephen Hemminger
<stephen@networkplumber.org> wrote:
> On Fri, 6 Feb 2015 16:51:56 -0800
> Mahesh Bandewar <maheshb@google.com> wrote:
>
>> @@ -396,6 +398,13 @@ static const struct bond_option bond_opts[BOND_OPT_LAST] = {
>> .values = bond_ad_actor_sysprio_tbl,
>> .set = bond_option_ad_actor_sysprio_set,
>> },
>> + [BOND_OPT_AD_ACTOR_SYS_MACADDR] = {
>> + .id = BOND_OPT_AD_ACTOR_SYS_MACADDR,
>> + .name = "ad_actor_system_mac_address",
>> + .unsuppmodes = BOND_MODE_ALL_EX(BIT(BOND_MODE_8023AD)),
>> + .flags = BOND_OPTFLAG_RAWVAL | BOND_OPTFLAG_IFDOWN,
>> + .set = bond_option_ad_actor_sys_macaddr_set,
>> + },
>> };
>
> I would consider the module command line options deprecated
> at this point. Why add more?
These are not the module parameters.
^ permalink raw reply
* Re: [PATCH v3] net: bluetooth: hci_sock: Use 'const u32 *' instead of 'void *' for 2nd parameter of hci_test_bit()
From: Chen Gang S @ 2015-02-08 0:00 UTC (permalink / raw)
To: Joe Perches
Cc: David Laight, Marcel Holtmann, Sergei Shtylyov,
Gustavo F. Padovan, Johan Hedberg, David S. Miller,
linux-bluetooth-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <1423338774.2933.9.camel-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>
On 2/8/15 03:52, Joe Perches wrote:
> On Sat, 2015-02-07 at 21:24 +0800, Chen Gang S wrote:
>> hci_test_bit() does not modify 2nd parameter, so it is better to let it
>> be constant, or may cause build warning. The related warning (with
>> allmodconfig under xtensa):
>>
>> net/bluetooth/hci_sock.c: In function 'hci_sock_sendmsg':
>> net/bluetooth/hci_sock.c:955:8: warning: passing argument 2 of 'hci_test_bit' discards 'const' qualifier from pointer target type [-Wdiscarded-array-qualifiers]
>> &hci_sec_filter.ocf_mask[ogf])) &&
>> ^
>> net/bluetooth/hci_sock.c:49:19: note: expected 'void *' but argument is of type 'const __u32 (*)[4] {aka const unsigned int (*)[4]}'
>> static inline int hci_test_bit(int nr, void *addr)
>> ^
>>
>> hci_test_bit() always treats 2nd parameter is u32, and all callers also
>> know about it, so 2nd parameter of hci_test_bit() need use 'const u32 *'
>> instead of 'void *'.
>>
>> C language treats the array function parameter as a pointer, so the
>> caller need not use '&' for the 2 demotion array, or it reports warning:
>> 'const unsigned int (*)[4]' is different with 'const unsigned int *'.
>
> I still think you are possibly papering over potential bugs
> on big-endian 64 bit systems.
>
> unsigned long vs u32.
>
> How are the bits actually set?
>
>From current usage of event_mask, "(u32 *) f->event_mask" is only for
event_mask data storage, not for calculation (always as "u32 *" for
calculation).
[root@localhost linux-next]# grep -rn "\<event_mask\>" include/net/bluetooth net/bluetooth
include/net/bluetooth/hci_sock.h:51: unsigned long event_mask[2];
include/net/bluetooth/hci_sock.h:57: __u32 event_mask[2];
net/bluetooth/hci_sock.c:59: __u32 event_mask[2];
net/bluetooth/hci_sock.c:110: if (!hci_test_bit(flt_event, (u32 *)&flt->event_mask))
net/bluetooth/hci_sock.c:1041: uf.event_mask[0] = *((u32 *) f->event_mask + 0);
net/bluetooth/hci_sock.c:1042: uf.event_mask[1] = *((u32 *) f->event_mask + 1);
net/bluetooth/hci_sock.c:1053: uf.event_mask[0] &= *((u32 *) hci_sec_filter.event_mask + 0);
net/bluetooth/hci_sock.c:1054: uf.event_mask[1] &= *((u32 *) hci_sec_filter.event_mask + 1);
net/bluetooth/hci_sock.c:1062: *((u32 *) f->event_mask + 0) = uf.event_mask[0];
net/bluetooth/hci_sock.c:1063: *((u32 *) f->event_mask + 1) = uf.event_mask[1];
net/bluetooth/hci_sock.c:1124: uf.event_mask[0] = *((u32 *) f->event_mask + 0);
net/bluetooth/hci_sock.c:1125: uf.event_mask[1] = *((u32 *) f->event_mask + 1);
Calculation is machine endian dependency, but event_mask is always as
"u32 *" for calculation, so there is no any type cast for calculation,
it is OK.
Storage is independent from machine endian, but it depends on machine
bits. In our case, 'unsigned long' array has enough space to accept u32
array, so there is no any data overwritten, it is OK.
By the way, I intended to remain event_mask as 'unsigned long' type,
because I am not quite sure whether it is also used by another modules
in kernel (or any other systems). May we change it to u32?
Thanks.
--
Chen Gang
Open, share, and attitude like air, water, and life which God blessed
^ permalink raw reply
* Re: [PATCH] vxlan: Wrong type passed to %pIS
From: Cong Wang @ 2015-02-07 23:31 UTC (permalink / raw)
To: Rasmus Villemoes
Cc: David S. Miller, Pravin B Shelar, Nicolas Dichtel,
Linux Kernel Network Developers, LKML
In-Reply-To: <87wq3tnbn0.fsf@rasmusvillemoes.dk>
On Sat, Feb 7, 2015 at 4:34 AM, Rasmus Villemoes
<linux@rasmusvillemoes.dk> wrote:
> On Sat, Feb 07 2015, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>
>> On Fri, Feb 6, 2015 at 6:17 PM, Rasmus Villemoes
>> <linux@rasmusvillemoes.dk> wrote:
>>> src_ip is a pointer to a union vxlan_addr, one member of which is a
>>> struct sockaddr. Passing a pointer to src_ip is wrong; one should pass
>>> the value of src_ip itself. Since %pIS formally expects something of
>>> type struct sockaddr*, let's pass a pointer to the appropriate union
>>> member, though this of course doesn't change the generated code.
>>>
>>
>>
>> It is a union, this doesn't harm.
>>
>
> Just to be clear: This fixes a real bug. The minimal fix had been
>
> - src_mac, &rdst->remote_ip, &src_ip);
> + src_mac, &rdst->remote_ip, src_ip);
>
> but I through in the cosmetic improvements while the line needed
> changing anyway.
>
Ah, I misread the patch.
Acked-by: Cong Wang <xiyou.wangcong@gmail.com>
Thanks!
^ permalink raw reply
* Re: [PATCH next 5/6] bonding: Allow userspace to set macaddr on bonding-dev
From: Stephen Hemminger @ 2015-02-07 22:26 UTC (permalink / raw)
To: Mahesh Bandewar
Cc: Jay Vosburgh, Andy Gospodarek, Veaceslav Falico,
Nikolay Aleksandrov, David Miller, netdev, Eric Dumazet
In-Reply-To: <1423270316-9311-1-git-send-email-maheshb@google.com>
On Fri, 6 Feb 2015 16:51:56 -0800
Mahesh Bandewar <maheshb@google.com> wrote:
> @@ -396,6 +398,13 @@ static const struct bond_option bond_opts[BOND_OPT_LAST] = {
> .values = bond_ad_actor_sysprio_tbl,
> .set = bond_option_ad_actor_sysprio_set,
> },
> + [BOND_OPT_AD_ACTOR_SYS_MACADDR] = {
> + .id = BOND_OPT_AD_ACTOR_SYS_MACADDR,
> + .name = "ad_actor_system_mac_address",
> + .unsuppmodes = BOND_MODE_ALL_EX(BIT(BOND_MODE_8023AD)),
> + .flags = BOND_OPTFLAG_RAWVAL | BOND_OPTFLAG_IFDOWN,
> + .set = bond_option_ad_actor_sys_macaddr_set,
> + },
> };
I would consider the module command line options deprecated
at this point. Why add more?
^ permalink raw reply
* Re: [PATCH 2/6] drivers: isdn: act2000: capi.c: fix checkpatch errors
From: Joe Perches @ 2015-02-07 21:04 UTC (permalink / raw)
To: Bas Peters
Cc: Paul Bolle, Tilman Schmidt, Sergei Shtylyov, isdn@linux-pingi.de,
Julia Lawall, David Miller, netdev, linux-kernel
In-Reply-To: <CAOgR63dbem_NGnMLsMCxBpkhp+QHSvVF5ozU-dvSFF34r5r6qA@mail.gmail.com>
On Sat, 2015-02-07 at 21:55 +0100, Bas Peters wrote:
> I thought it might have been
> useful and a good way to learn
Maybe pick a device you have (or maybe buy
something in the staging directory like a realtek
wireless device) and play with adding support for
something in it.
^ permalink raw reply
* Re: [PATCH 2/6] drivers: isdn: act2000: capi.c: fix checkpatch errors
From: Bas Peters @ 2015-02-07 20:55 UTC (permalink / raw)
To: Paul Bolle
Cc: Joe Perches, Tilman Schmidt, Sergei Shtylyov, isdn@linux-pingi.de,
Julia Lawall, David Miller, netdev, linux-kernel
In-Reply-To: <1423341800.2246.68.camel@x220>
2015-02-07 21:43 GMT+01:00 Paul Bolle <pebolle@tiscali.nl>:
> [Adding Tilman.]
>
> On Sat, 2015-02-07 at 11:19 -0800, Joe Perches wrote:
>> Does anyone still use these cards?
>
> 0) Good question.
>
> 1) None of the (two dozen) commits in drivers/isdn/act2000/ added since
> v2.6.12 appear to be triggered complaints, suggestions, etc. of actual
> users.
>
> 2) Broader picture: if I remember correctly there are now four different
> flavors of ISDN in the kernel:
> - really old: pre-i4l
> - very old: i4l
> - just old: CAPI
> - not so old: mISDN
>
> I could spend another 24 hours refining and relabeling these categories,
> and matching the various drivers to these categories. But I'm pretty
> sure none of the current categories contain all the drivers. And I'm
> certain all current categories support one or more drivers that none of
> the other categories do. So the current ISDN situation is a bit messy.
>
> Tilman might be able to provide a clearer, and maybe less grumpy,
> summary of the current situation.
>
> 3) Anyhow, this is an i4l card. i4l is deprecated since v2.6.22. And it
> was obsolete before that!
>
> 4) Furthermore, it seems consumer grade ISDN is on the way out. Eg, my
> ISP will disconnect my, let's call it, ADSL over ISDN connection in less
> that two months because they can't be bothered anymore to support that
> niche.
>
> 5) So we could let the various flavors of ISDN limp along for another
> few years. But maybe we should consider, say, removing i4l and pre i4l
> and see who complains. That might be a rude thing to do. So perhaps the
> various ISDN flavors should be left alone until ... what exactly?
>
>
> Paul Bolle
>
In that case I will drop this patchset. I thought it might have been
useful and a good way to learn
but I'm just wasting everyone's time with it if it's that deprecated.
Regards,
Bas
^ permalink raw reply
* Re: [PATCH 2/6] drivers: isdn: act2000: capi.c: fix checkpatch errors
From: Paul Bolle @ 2015-02-07 20:43 UTC (permalink / raw)
To: Joe Perches
Cc: Tilman Schmidt, Sergei Shtylyov, Bas Peters, isdn, julia.lawall,
davem, netdev, linux-kernel
In-Reply-To: <1423336761.2933.7.camel@perches.com>
[Adding Tilman.]
On Sat, 2015-02-07 at 11:19 -0800, Joe Perches wrote:
> Does anyone still use these cards?
0) Good question.
1) None of the (two dozen) commits in drivers/isdn/act2000/ added since
v2.6.12 appear to be triggered complaints, suggestions, etc. of actual
users.
2) Broader picture: if I remember correctly there are now four different
flavors of ISDN in the kernel:
- really old: pre-i4l
- very old: i4l
- just old: CAPI
- not so old: mISDN
I could spend another 24 hours refining and relabeling these categories,
and matching the various drivers to these categories. But I'm pretty
sure none of the current categories contain all the drivers. And I'm
certain all current categories support one or more drivers that none of
the other categories do. So the current ISDN situation is a bit messy.
Tilman might be able to provide a clearer, and maybe less grumpy,
summary of the current situation.
3) Anyhow, this is an i4l card. i4l is deprecated since v2.6.22. And it
was obsolete before that!
4) Furthermore, it seems consumer grade ISDN is on the way out. Eg, my
ISP will disconnect my, let's call it, ADSL over ISDN connection in less
that two months because they can't be bothered anymore to support that
niche.
5) So we could let the various flavors of ISDN limp along for another
few years. But maybe we should consider, say, removing i4l and pre i4l
and see who complains. That might be a rude thing to do. So perhaps the
various ISDN flavors should be left alone until ... what exactly?
Paul Bolle
^ permalink raw reply
* Please respond!!!
From: Antonio Vinal @ 2015-02-05 20:47 UTC (permalink / raw)
Hello friend!
I would like to contact you personally for an important proposal that could of interest to you.
I send this email only to know if this email address is functional.
I have something very important to discuss with you. Contact me for details by: Email: fernrodyup12@aol.jp with your direct contacts.
Kind regards.
Antonio Vi?al.
^ permalink raw reply
* Re: [PATCH v3] net: bluetooth: hci_sock: Use 'const u32 *' instead of 'void *' for 2nd parameter of hci_test_bit()
From: Joe Perches @ 2015-02-07 19:52 UTC (permalink / raw)
To: Chen Gang S
Cc: David Laight, Marcel Holtmann, Sergei Shtylyov,
Gustavo F. Padovan, Johan Hedberg, David S. Miller,
linux-bluetooth@vger.kernel.org, linux-kernel@vger.kernel.org,
netdev@vger.kernel.org
In-Reply-To: <54D61229.9010904@sunrus.com.cn>
On Sat, 2015-02-07 at 21:24 +0800, Chen Gang S wrote:
> hci_test_bit() does not modify 2nd parameter, so it is better to let it
> be constant, or may cause build warning. The related warning (with
> allmodconfig under xtensa):
>
> net/bluetooth/hci_sock.c: In function 'hci_sock_sendmsg':
> net/bluetooth/hci_sock.c:955:8: warning: passing argument 2 of 'hci_test_bit' discards 'const' qualifier from pointer target type [-Wdiscarded-array-qualifiers]
> &hci_sec_filter.ocf_mask[ogf])) &&
> ^
> net/bluetooth/hci_sock.c:49:19: note: expected 'void *' but argument is of type 'const __u32 (*)[4] {aka const unsigned int (*)[4]}'
> static inline int hci_test_bit(int nr, void *addr)
> ^
>
> hci_test_bit() always treats 2nd parameter is u32, and all callers also
> know about it, so 2nd parameter of hci_test_bit() need use 'const u32 *'
> instead of 'void *'.
>
> C language treats the array function parameter as a pointer, so the
> caller need not use '&' for the 2 demotion array, or it reports warning:
> 'const unsigned int (*)[4]' is different with 'const unsigned int *'.
I still think you are possibly papering over potential bugs
on big-endian 64 bit systems.
unsigned long vs u32.
How are the bits actually set?
^ permalink raw reply
* Re: [PATCH next 6/6] bonding: Implement user key part of port_key in an AD system.
From: Mahesh Bandewar @ 2015-02-07 19:29 UTC (permalink / raw)
To: Andy Gospodarek
Cc: Jay Vosburgh, Andy Gospodarek, Veaceslav Falico,
Nikolay Aleksandrov, David Miller, netdev, Eric Dumazet
In-Reply-To: <20150207034029.GE34197@gospo.home.greyhouse.net>
On Fri, Feb 6, 2015 at 7:40 PM, Andy Gospodarek
<gospo@cumulusnetworks.com> wrote:
> On Fri, Feb 06, 2015 at 04:51:57PM -0800, Mahesh Bandewar wrote:
>> The port key has three components - user-key, speed-part, and duplex-part.
>> The LSBit is for the duplex-part, next 5 bits are for the speed while the
>> remaining 10 bits are the user defined key bits. Get these 10 bits
>> from the user-space (through the SysFs interface) and use it to form the
>> admin port-key. Allowed range for the user-key is 0 - 1023 (10 bits). If
>> it is not provided then use zero for the user-key-bits (default).
>>
>> It can set using following example code -
>>
>> # modprobe bonding mode=4
>> # usr_port_key=$(( RANDOM & 0x3FF ))
>> # echo $usr_port_key > /sys/class/net/bond0/bonding/ad_actor_user_port_key
>> # echo +eth1 > /sys/class/net/bond0/bonding/slaves
>
> If the other ones don't have 'actor' maybe drop it here too.
>
> (Sorry to be picky about these, but as someone who has typed many of
> these options a bunch it would be great to have shorter names in config
> files.)
>
Thanks for the suggestions Andy. Now I don't have to battle with
myself for the correct names :) I wanted to move the naming discussion
in the other thread where I have proposed this to be changed to
'ad_user_portkey'.
>> ...
>> # ip link set bond0 up
>>
>> Signed-off-by: Mahesh Bandewar <maheshb@google.com>
>> ---
>> drivers/net/bonding/bond_3ad.c | 6 +++---
>> drivers/net/bonding/bond_main.c | 10 ++++++++++
>> drivers/net/bonding/bond_options.c | 26 ++++++++++++++++++++++++++
>> drivers/net/bonding/bond_sysfs.c | 15 +++++++++++++++
>> include/net/bond_options.h | 1 +
>> include/net/bonding.h | 1 +
>> 6 files changed, 56 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
>> index 373d3db3809f..c69c393ba8c5 100644
>> --- a/drivers/net/bonding/bond_3ad.c
>> +++ b/drivers/net/bonding/bond_3ad.c
>> @@ -1955,10 +1955,10 @@ void bond_3ad_bind_slave(struct slave *slave)
>>
>> port->slave = slave;
>> port->actor_port_number = SLAVE_AD_INFO(slave)->id;
>> - /* key is determined according to the link speed, duplex and user key(which
>> - * is yet not supported)
>> + /* key is determined according to the link speed, duplex and
>> + * user key
>> */
>> - port->actor_admin_port_key = 0;
>> + port->actor_admin_port_key = bond->params.ad_actor_portkey << 6;
>> port->actor_admin_port_key |= __get_duplex(port);
>> port->actor_admin_port_key |= (__get_link_speed(port) << 1);
>> port->actor_oper_port_key = port->actor_admin_port_key;
>> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>> index 1a6735ef2ea7..f9f001f35a91 100644
>> --- a/drivers/net/bonding/bond_main.c
>> +++ b/drivers/net/bonding/bond_main.c
>> @@ -4105,6 +4105,7 @@ static int bond_check_params(struct bond_params *params)
>> const struct bond_opt_value *valptr;
>> int arp_all_targets_value;
>> u16 ad_actor_sysprio = 0;
>> + u16 ad_actor_portkey = 0;
>>
>> /* Convert string parameters. */
>> if (mode) {
>> @@ -4409,6 +4410,14 @@ static int bond_check_params(struct bond_params *params)
>> return -EINVAL;
>> }
>> ad_actor_sysprio = valptr->value;
>> +
>> + valptr = bond_opt_parse(bond_opt_get(BOND_OPT_AD_ACTOR_PORTKEY),
>> + &newval);
>> + if (!valptr) {
>> + pr_err("Error: No ad_actor_portkey default value");
>> + return -EINVAL;
>> + }
>> + ad_actor_portkey = valptr->value;
>> }
>>
>> if (lp_interval == 0) {
>> @@ -4441,6 +4450,7 @@ static int bond_check_params(struct bond_params *params)
>> params->tlb_dynamic_lb = 1; /* Default value */
>> params->ad_actor_sysprio = ad_actor_sysprio;
>> eth_zero_addr(params->ad_actor_sys_macaddr);
>> + params->ad_actor_portkey = ad_actor_portkey;
>> if (packets_per_slave > 0) {
>> params->reciprocal_packets_per_slave =
>> reciprocal_value(packets_per_slave);
>> diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
>> index 330d48b6a1e6..c3f77c6d0d2e 100644
>> --- a/drivers/net/bonding/bond_options.c
>> +++ b/drivers/net/bonding/bond_options.c
>> @@ -74,6 +74,8 @@ static int bond_option_ad_actor_sysprio_set(struct bonding *bond,
>> const struct bond_opt_value *newval);
>> static int bond_option_ad_actor_sys_macaddr_set(struct bonding *bond,
>> const struct bond_opt_value *newval);
>> +static int bond_option_ad_actor_portkey_set(struct bonding *bond,
>> + const struct bond_opt_value *newval);
>>
>>
>> static const struct bond_opt_value bond_mode_tbl[] = {
>> @@ -196,6 +198,12 @@ static const struct bond_opt_value bond_ad_actor_sysprio_tbl[] = {
>> { NULL, -1, 0},
>> };
>>
>> +static const struct bond_opt_value bond_ad_actor_portkey_tbl[] = {
>> + { "minval", 0, BOND_VALFLAG_MIN | BOND_VALFLAG_DEFAULT},
>> + { "maxval", 1023, BOND_VALFLAG_MAX},
>> + { NULL, -1, 0},
>> +};
>> +
>> static const struct bond_option bond_opts[BOND_OPT_LAST] = {
>> [BOND_OPT_MODE] = {
>> .id = BOND_OPT_MODE,
>> @@ -405,6 +413,14 @@ static const struct bond_option bond_opts[BOND_OPT_LAST] = {
>> .flags = BOND_OPTFLAG_RAWVAL | BOND_OPTFLAG_IFDOWN,
>> .set = bond_option_ad_actor_sys_macaddr_set,
>> },
>> + [BOND_OPT_AD_ACTOR_PORTKEY] = {
>> + .id = BOND_OPT_AD_ACTOR_PORTKEY,
>> + .name = "ad_actor_user_port_key",
>> + .unsuppmodes = BOND_MODE_ALL_EX(BIT(BOND_MODE_8023AD)),
>> + .flags = BOND_OPTFLAG_IFDOWN,
>> + .values = bond_ad_actor_portkey_tbl,
>> + .set = bond_option_ad_actor_portkey_set,
>> + }
>> };
>>
>> /* Searches for an option by name */
>> @@ -1405,3 +1421,13 @@ static int bond_option_ad_actor_sys_macaddr_set(struct bonding *bond,
>>
>> return 0;
>> }
>> +
>> +static int bond_option_ad_actor_portkey_set(struct bonding *bond,
>> + const struct bond_opt_value *newval)
>> +{
>> + netdev_info(bond->dev, "Setting ad_actor_portkey to (%llu)\n",
>> + newval->value);
>> +
>> + bond->params.ad_actor_portkey = newval->value;
>> + return 0;
>> +}
>> diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
>> index 91713d0b6685..17d84d5a1608 100644
>> --- a/drivers/net/bonding/bond_sysfs.c
>> +++ b/drivers/net/bonding/bond_sysfs.c
>> @@ -721,6 +721,20 @@ static DEVICE_ATTR(ad_actor_system_mac_address, S_IRUGO | S_IWUSR,
>> bonding_show_ad_actor_sys_macaddr,
>> bonding_sysfs_store_option);
>>
>> +static ssize_t bonding_show_ad_actor_portkey(struct device *d,
>> + struct device_attribute *attr,
>> + char *buf)
>> +{
>> + struct bonding *bond = to_bond(d);
>> +
>> + if (BOND_MODE(bond) == BOND_MODE_8023AD)
>> + return sprintf(buf, "%hu\n", bond->params.ad_actor_portkey);
>> +
>> + return 0;
>> +}
>> +static DEVICE_ATTR(ad_actor_user_port_key, S_IRUGO | S_IWUSR,
>> + bonding_show_ad_actor_portkey, bonding_sysfs_store_option);
>> +
>> static struct attribute *per_bond_attrs[] = {
>> &dev_attr_slaves.attr,
>> &dev_attr_mode.attr,
>> @@ -756,6 +770,7 @@ static struct attribute *per_bond_attrs[] = {
>> &dev_attr_tlb_dynamic_lb.attr,
>> &dev_attr_ad_actor_system_priority.attr,
>> &dev_attr_ad_actor_system_mac_address.attr,
>> + &dev_attr_ad_actor_user_port_key.attr,
>> NULL,
>> };
>>
>> diff --git a/include/net/bond_options.h b/include/net/bond_options.h
>> index 993ef73cd050..4c36455aacb4 100644
>> --- a/include/net/bond_options.h
>> +++ b/include/net/bond_options.h
>> @@ -65,6 +65,7 @@ enum {
>> BOND_OPT_TLB_DYNAMIC_LB,
>> BOND_OPT_AD_ACTOR_SYSPRIO,
>> BOND_OPT_AD_ACTOR_SYS_MACADDR,
>> + BOND_OPT_AD_ACTOR_PORTKEY,
>> BOND_OPT_LAST
>> };
>>
>> diff --git a/include/net/bonding.h b/include/net/bonding.h
>> index cd2092e6bc71..e91db2566437 100644
>> --- a/include/net/bonding.h
>> +++ b/include/net/bonding.h
>> @@ -144,6 +144,7 @@ struct bond_params {
>> int tlb_dynamic_lb;
>> struct reciprocal_value reciprocal_packets_per_slave;
>> u16 ad_actor_sysprio;
>> + u16 ad_actor_portkey;
>> u8 ad_actor_sys_macaddr[ETH_ALEN];
>> };
>>
>> --
>> 2.2.0.rc0.207.ga3a616c
>>
>> --
>> 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 next 4/6] bonding: Allow userspace to set system_priority
From: Mahesh Bandewar @ 2015-02-07 19:26 UTC (permalink / raw)
To: Jay Vosburgh
Cc: Andy Gospodarek, Andy Gospodarek, Veaceslav Falico,
Nikolay Aleksandrov, David Miller, netdev, Eric Dumazet
In-Reply-To: <6873.1423289974@famine>
On Fri, Feb 6, 2015 at 10:19 PM, Jay Vosburgh
<jay.vosburgh@canonical.com> wrote:
> Andy Gospodarek <gospo@cumulusnetworks.com> wrote:
>
>>On Fri, Feb 06, 2015 at 04:51:54PM -0800, Mahesh Bandewar wrote:
>>> This patch allows user to randomize the system-priority in an ad-system.
>>> The allowed range is 1 - 0xFFFF while default value is 0xFFFF. If user
>>> does not specify this value, the system defaults to 0xFFFF, which is
>>> what it was before this patch.
>>>
>>> Following example code could set the value -
>>> # modprobe bonding mode=4
>>> # sys_prio=$(( 1 + RANDOM + RANDOM ))
>>> # echo $sys_prio > /sys/class/net/bond0/bonding/ad_actor_system_priority
>>
>>If I can convince you to change 'ad_actor_system_macaddr' to something
>>different can I also convince you to change this to something shorter,
>>too? :)
>>
>>Maybe 'ad_sys_priority' or something?
>
> The name, verbose as it is, is from the 802.1AX standard, and
> there's also a "partner_system_priority" (which is not a user-settable
> thing, it's a field in the LACPDUs). My suggestion would therefore be
> "ad_actor_sys_prio" for this one, as I think there's some value in
> continuity with the names from the standard.
>
> The MAC address one in the standard is just "actor_system";
> there's a "partner_system" here, too, which is also a field in the
> LACPDU. I'm ok with calling that one just "actor_system," as presumably
> anyone changing it will know what it means.
>
Thank you guys for the suggestions. I didn't like those very long
names either but when there is something that already has name
similar, I defaulted to being verbose. I will have the name changed to
-
ad_actor_system_priority - ad_actor_sys_prio
ad_actor_system_mac_address - ad_actor_system
ad_actor_user_port_key - ad_user_portkey
Is this reasonable enough?
> -J
>
>>Thanks!
>>
>>> # echo +eth1 > /sys/class/net/bond0/bonding/slaves
>>> ...
>>> # ip link set bond0 up
>>>
>>> Signed-off-by: Mahesh Bandewar <maheshb@google.com>
>>> ---
>>> drivers/net/bonding/bond_3ad.c | 5 ++++-
>>> drivers/net/bonding/bond_main.c | 14 ++++++++++++++
>>> drivers/net/bonding/bond_options.c | 29 ++++++++++++++++++++++++++++-
>>> drivers/net/bonding/bond_procfs.c | 2 ++
>>> drivers/net/bonding/bond_sysfs.c | 15 +++++++++++++++
>>> include/net/bond_options.h | 1 +
>>> include/net/bonding.h | 1 +
>>> 7 files changed, 65 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
>>> index 2a69095266c1..1177f96194dd 100644
>>> --- a/drivers/net/bonding/bond_3ad.c
>>> +++ b/drivers/net/bonding/bond_3ad.c
>>> @@ -1912,7 +1912,8 @@ void bond_3ad_initialize(struct bonding *bond, u16 tick_resolution)
>>>
>>> BOND_AD_INFO(bond).aggregator_identifier = 0;
>>>
>>> - BOND_AD_INFO(bond).system.sys_priority = 0xFFFF;
>>> + BOND_AD_INFO(bond).system.sys_priority =
>>> + bond->params.ad_actor_sysprio;
>>> BOND_AD_INFO(bond).system.sys_mac_addr = *((struct mac_addr *)bond->dev->dev_addr);
>>>
>>> /* initialize how many times this module is called in one
>>> @@ -1963,6 +1964,8 @@ void bond_3ad_bind_slave(struct slave *slave)
>>> port->sm_vars &= ~AD_PORT_LACP_ENABLED;
>>> /* actor system is the bond's system */
>>> port->actor_system = BOND_AD_INFO(bond).system.sys_mac_addr;
>>> + port->actor_system_priority =
>>> + BOND_AD_INFO(bond).system.sys_priority;
>>> /* tx timer(to verify that no more than MAX_TX_IN_SECOND
>>> * lacpdu's are sent in one second)
>>> */
>>> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>>> index a50ec87486f3..561b2bde5aeb 100644
>>> --- a/drivers/net/bonding/bond_main.c
>>> +++ b/drivers/net/bonding/bond_main.c
>>> @@ -4104,6 +4104,7 @@ static int bond_check_params(struct bond_params *params)
>>> struct bond_opt_value newval;
>>> const struct bond_opt_value *valptr;
>>> int arp_all_targets_value;
>>> + u16 ad_actor_sysprio = 0;
>>>
>>> /* Convert string parameters. */
>>> if (mode) {
>>> @@ -4398,6 +4399,18 @@ static int bond_check_params(struct bond_params *params)
>>> fail_over_mac_value = BOND_FOM_NONE;
>>> }
>>>
>>> + if (bond_mode == BOND_MODE_8023AD) {
>>> + bond_opt_initstr(&newval, "default");
>>> + valptr = bond_opt_parse(
>>> + bond_opt_get(BOND_OPT_AD_ACTOR_SYSPRIO),
>>> + &newval);
>>> + if (!valptr) {
>>> + pr_err("Error: No ad_actor_sysprio default value");
>>> + return -EINVAL;
>>> + }
>>> + ad_actor_sysprio = valptr->value;
>>> + }
>>> +
>>> if (lp_interval == 0) {
>>> pr_warn("Warning: ip_interval must be between 1 and %d, so it was reset to %d\n",
>>> INT_MAX, BOND_ALB_DEFAULT_LP_INTERVAL);
>>> @@ -4426,6 +4439,7 @@ static int bond_check_params(struct bond_params *params)
>>> params->lp_interval = lp_interval;
>>> params->packets_per_slave = packets_per_slave;
>>> params->tlb_dynamic_lb = 1; /* Default value */
>>> + params->ad_actor_sysprio = ad_actor_sysprio;
>>> if (packets_per_slave > 0) {
>>> params->reciprocal_packets_per_slave =
>>> reciprocal_value(packets_per_slave);
>>> diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
>>> index 4df28943d222..d8f6760143ae 100644
>>> --- a/drivers/net/bonding/bond_options.c
>>> +++ b/drivers/net/bonding/bond_options.c
>>> @@ -70,6 +70,8 @@ static int bond_option_slaves_set(struct bonding *bond,
>>> const struct bond_opt_value *newval);
>>> static int bond_option_tlb_dynamic_lb_set(struct bonding *bond,
>>> const struct bond_opt_value *newval);
>>> +static int bond_option_ad_actor_sysprio_set(struct bonding *bond,
>>> + const struct bond_opt_value *newval);
>>>
>>>
>>> static const struct bond_opt_value bond_mode_tbl[] = {
>>> @@ -186,6 +188,12 @@ static const struct bond_opt_value bond_tlb_dynamic_lb_tbl[] = {
>>> { NULL, -1, 0}
>>> };
>>>
>>> +static const struct bond_opt_value bond_ad_actor_sysprio_tbl[] = {
>>> + { "minval", 1, BOND_VALFLAG_MIN},
>>> + { "maxval", 65535, BOND_VALFLAG_MAX | BOND_VALFLAG_DEFAULT},
>>> + { NULL, -1, 0},
>>> +};
>>> +
>>> static const struct bond_option bond_opts[BOND_OPT_LAST] = {
>>> [BOND_OPT_MODE] = {
>>> .id = BOND_OPT_MODE,
>>> @@ -379,7 +387,15 @@ static const struct bond_option bond_opts[BOND_OPT_LAST] = {
>>> .values = bond_tlb_dynamic_lb_tbl,
>>> .flags = BOND_OPTFLAG_IFDOWN,
>>> .set = bond_option_tlb_dynamic_lb_set,
>>> - }
>>> + },
>>> + [BOND_OPT_AD_ACTOR_SYSPRIO] = {
>>> + .id = BOND_OPT_AD_ACTOR_SYSPRIO,
>>> + .name = "ad_actor_system_priority",
>>> + .unsuppmodes = BOND_MODE_ALL_EX(BIT(BOND_MODE_8023AD)),
>>> + .flags = BOND_OPTFLAG_IFDOWN,
>>> + .values = bond_ad_actor_sysprio_tbl,
>>> + .set = bond_option_ad_actor_sysprio_set,
>>> + },
>>> };
>>>
>>> /* Searches for an option by name */
>>> @@ -1349,3 +1365,14 @@ static int bond_option_tlb_dynamic_lb_set(struct bonding *bond,
>>>
>>> return 0;
>>> }
>>> +
>>> +
>>> +static int bond_option_ad_actor_sysprio_set(struct bonding *bond,
>>> + const struct bond_opt_value *newval)
>>> +{
>>> + netdev_info(bond->dev, "Setting ad_actor_sysprio to (%llu)\n",
>>> + newval->value);
>>> +
>>> + bond->params.ad_actor_sysprio = newval->value;
>>> + return 0;
>>> +}
>>> diff --git a/drivers/net/bonding/bond_procfs.c b/drivers/net/bonding/bond_procfs.c
>>> index 83095a0b4b90..9e33c48886ef 100644
>>> --- a/drivers/net/bonding/bond_procfs.c
>>> +++ b/drivers/net/bonding/bond_procfs.c
>>> @@ -134,6 +134,8 @@ static void bond_info_show_master(struct seq_file *seq)
>>> bond->params.ad_select);
>>> seq_printf(seq, "Aggregator selection policy (ad_select): %s\n",
>>> optval->string);
>>> + seq_printf(seq, "System priority: %d\n",
>>> + BOND_AD_INFO(bond).system.sys_priority);
>>>
>>> if (__bond_3ad_get_active_agg_info(bond, &ad_info)) {
>>> seq_printf(seq, "bond %s has no active aggregator\n",
>>> diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
>>> index 7e9e151d4d61..4350aa06f867 100644
>>> --- a/drivers/net/bonding/bond_sysfs.c
>>> +++ b/drivers/net/bonding/bond_sysfs.c
>>> @@ -692,6 +692,20 @@ static ssize_t bonding_show_packets_per_slave(struct device *d,
>>> static DEVICE_ATTR(packets_per_slave, S_IRUGO | S_IWUSR,
>>> bonding_show_packets_per_slave, bonding_sysfs_store_option);
>>>
>>> +static ssize_t bonding_show_ad_actor_sysprio(struct device *d,
>>> + struct device_attribute *attr,
>>> + char *buf)
>>> +{
>>> + struct bonding *bond = to_bond(d);
>>> +
>>> + if (BOND_MODE(bond) == BOND_MODE_8023AD)
>>> + return sprintf(buf, "%hu\n", bond->params.ad_actor_sysprio);
>>> +
>>> + return 0;
>>> +}
>>> +static DEVICE_ATTR(ad_actor_system_priority, S_IRUGO | S_IWUSR,
>>> + bonding_show_ad_actor_sysprio, bonding_sysfs_store_option);
>>> +
>>> static struct attribute *per_bond_attrs[] = {
>>> &dev_attr_slaves.attr,
>>> &dev_attr_mode.attr,
>>> @@ -725,6 +739,7 @@ static struct attribute *per_bond_attrs[] = {
>>> &dev_attr_lp_interval.attr,
>>> &dev_attr_packets_per_slave.attr,
>>> &dev_attr_tlb_dynamic_lb.attr,
>>> + &dev_attr_ad_actor_system_priority.attr,
>>> NULL,
>>> };
>>>
>>> diff --git a/include/net/bond_options.h b/include/net/bond_options.h
>>> index ea6546d2c946..c2af1db37354 100644
>>> --- a/include/net/bond_options.h
>>> +++ b/include/net/bond_options.h
>>> @@ -63,6 +63,7 @@ enum {
>>> BOND_OPT_LP_INTERVAL,
>>> BOND_OPT_SLAVES,
>>> BOND_OPT_TLB_DYNAMIC_LB,
>>> + BOND_OPT_AD_ACTOR_SYSPRIO,
>>> BOND_OPT_LAST
>>> };
>>>
>>> diff --git a/include/net/bonding.h b/include/net/bonding.h
>>> index 29f53eacac0a..7a5c79fcf866 100644
>>> --- a/include/net/bonding.h
>>> +++ b/include/net/bonding.h
>>> @@ -143,6 +143,7 @@ struct bond_params {
>>> int packets_per_slave;
>>> int tlb_dynamic_lb;
>>> struct reciprocal_value reciprocal_packets_per_slave;
>>> + u16 ad_actor_sysprio;
>>> };
>>>
>>> struct bond_parm_tbl {
>>> --
>>> 2.2.0.rc0.207.ga3a616c
>
> ---
> -Jay Vosburgh, jay.vosburgh@canonical.com
^ permalink raw reply
* Re: [PATCH 2/6] drivers: isdn: act2000: capi.c: fix checkpatch errors
From: Joe Perches @ 2015-02-07 19:19 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: Bas Peters, isdn, julia.lawall, davem, netdev, linux-kernel
In-Reply-To: <54D650BF.5000400@cogentembedded.com>
On Sat, 2015-02-07 at 20:51 +0300, Sergei Shtylyov wrote:
> On 02/07/2015 08:06 PM, Bas Peters wrote:
>
> > This patch fixes the following checkpatch errors:
> > 1. trailing statement
> > 1. assignment of variable in if condition
> > 1. incorrectly placed brace after function definition
I wonder about the usefulness of these changes.
Does anyone still use these cards?
> > diff --git a/drivers/isdn/act2000/capi.c b/drivers/isdn/act2000/capi.c
[]
> > @@ -113,7 +113,8 @@ actcapi_chkhdr(act2000_card *card, actcapi_msghdr *hdr)
> > m->hdr.cmd.cmd = c; \
> > m->hdr.cmd.subcmd = s; \
> > m->hdr.msgnum = actcapi_nextsmsg(card); \
> > - } else m = NULL; \
> > + } else
> > + m = NULL; \
>
> Documentation/CodingStyle has an extra rule for such case: *else* branch
> should also have {} since *if* branch has {}.
The macro itself is already poor form.
-----
#define ACTCAPI_MKHDR(l, c, s) { \
skb = alloc_skb(l + 8, GFP_ATOMIC); \
if (skb) { \
m = (actcapi_msg *)skb_put(skb, l + 8); \
m->hdr.len = l + 8; \
m->hdr.applicationID = 1; \
m->hdr.cmd.cmd = c; \
m->hdr.cmd.subcmd = s; \
m->hdr.msgnum = actcapi_nextsmsg(card); \
} else m = NULL; \
}
-----
o hidden arguments skb, m and card
o missing do {} while around multi-statement macro
It'd probably be nicer to change the macro to a function
and pass m and card.
This would reduce the object size too.
But maybe any change here is not necessary.
If anything is done, I suggest something like:
---
drivers/isdn/act2000/capi.c | 204 +++++++++++++++++++++++++-------------------
1 file changed, 115 insertions(+), 89 deletions(-)
diff --git a/drivers/isdn/act2000/capi.c b/drivers/isdn/act2000/capi.c
index 3f66ca2..a7ecf51 100644
--- a/drivers/isdn/act2000/capi.c
+++ b/drivers/isdn/act2000/capi.c
@@ -104,29 +104,36 @@ actcapi_chkhdr(act2000_card *card, actcapi_msghdr *hdr)
return 0;
}
-#define ACTCAPI_MKHDR(l, c, s) { \
- skb = alloc_skb(l + 8, GFP_ATOMIC); \
- if (skb) { \
- m = (actcapi_msg *)skb_put(skb, l + 8); \
- m->hdr.len = l + 8; \
- m->hdr.applicationID = 1; \
- m->hdr.cmd.cmd = c; \
- m->hdr.cmd.subcmd = s; \
- m->hdr.msgnum = actcapi_nextsmsg(card); \
- } else m = NULL; \
- }
+static struct sk_buff *actcapi_mkhdr(act2000_card *card, actcapi_msg **m,
+ int len, int cmd, int subcmd)
+{
+ struct sk_buff *skb;
-#define ACTCAPI_CHKSKB if (!skb) { \
- printk(KERN_WARNING "actcapi: alloc_skb failed\n"); \
- return; \
- }
+ len += 8;
-#define ACTCAPI_QUEUE_TX { \
- actcapi_debug_msg(skb, 1); \
- skb_queue_tail(&card->sndq, skb); \
- act2000_schedule_tx(card); \
+ skb = alloc_skb(len, GFP_ATOMIC);
+ if (!skb) {
+ *m = NULL;
+ return NULL;
}
+ *m = (actcapi_msg *)skb_put(skb, len);
+ (*m)->hdr.len = len;
+ (*m)->hdr.applicationID = 1;
+ (*m)->hdr.cmd.cmd = cmd;
+ (*m)->hdr.cmd.subcmd = subcmd;
+ (*m)->hdr.msgnum = actcapi_nextsmsg(card);
+
+ return skb;
+}
+
+static void actcapi_queue_tx(act2000_card *card, struct sk_buff *skb)
+{
+ actcapi_debug_msg(skb, 1);
+ skb_queue_tail(&card->sndq, skb);
+ act2000_schedule_tx(card);
+}
+
int
actcapi_listen_req(act2000_card *card)
{
@@ -137,16 +144,15 @@ actcapi_listen_req(act2000_card *card)
for (i = 0; i < ACT2000_BCH; i++)
eazmask |= card->bch[i].eazmask;
- ACTCAPI_MKHDR(9, 0x05, 0x00);
- if (!skb) {
- printk(KERN_WARNING "actcapi: alloc_skb failed\n");
+ skb = actcapi_mkhdr(card, &m, 9, 0x05, 0x00);
+ if (!skb)
return -ENOMEM;
- }
+
m->msg.listen_req.controller = 0;
m->msg.listen_req.infomask = 0x3f; /* All information */
m->msg.listen_req.eazmask = eazmask;
m->msg.listen_req.simask = (eazmask) ? 0x86 : 0; /* All SI's */
- ACTCAPI_QUEUE_TX;
+ actcapi_queue_tx(card, skb);
return 0;
}
@@ -157,12 +163,12 @@ actcapi_connect_req(act2000_card *card, act2000_chan *chan, char *phone,
actcapi_msg *m;
struct sk_buff *skb;
- ACTCAPI_MKHDR((11 + strlen(phone)), 0x02, 0x00);
+ skb = actcapi_mkhdr(card, &m, 11 + strlen(phone), 0x02, 0x00);
if (!skb) {
- printk(KERN_WARNING "actcapi: alloc_skb failed\n");
chan->fsm_state = ACT2000_STATE_NULL;
return -ENOMEM;
}
+
m->msg.connect_req.controller = 0;
m->msg.connect_req.bchan = 0x83;
m->msg.connect_req.infomask = 0x3f;
@@ -173,7 +179,7 @@ actcapi_connect_req(act2000_card *card, act2000_chan *chan, char *phone,
m->msg.connect_req.addr.tnp = 0x81;
memcpy(m->msg.connect_req.addr.num, phone, strlen(phone));
chan->callref = m->hdr.msgnum;
- ACTCAPI_QUEUE_TX;
+ actcapi_queue_tx(card, skb);
return 0;
}
@@ -183,14 +189,16 @@ actcapi_connect_b3_req(act2000_card *card, act2000_chan *chan)
actcapi_msg *m;
struct sk_buff *skb;
- ACTCAPI_MKHDR(17, 0x82, 0x00);
- ACTCAPI_CHKSKB;
+ skb = actcapi_mkhdr(card, &m, 17, 0x82, 0x00);
+ if (!skb)
+ return;
+
m->msg.connect_b3_req.plci = chan->plci;
memset(&m->msg.connect_b3_req.ncpi, 0,
sizeof(m->msg.connect_b3_req.ncpi));
m->msg.connect_b3_req.ncpi.len = 13;
m->msg.connect_b3_req.ncpi.modulo = 8;
- ACTCAPI_QUEUE_TX;
+ actcapi_queue_tx(card, skb);
}
/*
@@ -202,15 +210,14 @@ actcapi_manufacturer_req_net(act2000_card *card)
actcapi_msg *m;
struct sk_buff *skb;
- ACTCAPI_MKHDR(5, 0xff, 0x00);
- if (!skb) {
- printk(KERN_WARNING "actcapi: alloc_skb failed\n");
+ skb = actcapi_mkhdr(card, &m, 5, 0xff, 0x00);
+ if (!skb)
return -ENOMEM;
- }
+
m->msg.manufacturer_req_net.manuf_msg = 0x11;
m->msg.manufacturer_req_net.controller = 1;
m->msg.manufacturer_req_net.nettype = (card->ptype == ISDN_PTYPE_EURO) ? 1 : 0;
- ACTCAPI_QUEUE_TX;
+ actcapi_queue_tx(card, skb);
printk(KERN_INFO "act2000 %s: D-channel protocol now %s\n",
card->interface.id, (card->ptype == ISDN_PTYPE_EURO) ? "euro" : "1tr6");
card->interface.features &=
@@ -230,16 +237,14 @@ actcapi_manufacturer_req_v42(act2000_card *card, ulong arg)
actcapi_msg *m;
struct sk_buff *skb;
- ACTCAPI_MKHDR(8, 0xff, 0x00);
- if (!skb) {
-
- printk(KERN_WARNING "actcapi: alloc_skb failed\n");
+ skb = actcapi_mkhdr(card, &m, 8, 0xff, 0x00);
+ if (!skb)
return -ENOMEM;
- }
+
m->msg.manufacturer_req_v42.manuf_msg = 0x10;
m->msg.manufacturer_req_v42.controller = 0;
m->msg.manufacturer_req_v42.v42control = (arg ? 1 : 0);
- ACTCAPI_QUEUE_TX;
+ actcapi_queue_tx(card, skb);
return 0;
}
#endif /* 0 */
@@ -253,15 +258,13 @@ actcapi_manufacturer_req_errh(act2000_card *card)
actcapi_msg *m;
struct sk_buff *skb;
- ACTCAPI_MKHDR(4, 0xff, 0x00);
- if (!skb) {
-
- printk(KERN_WARNING "actcapi: alloc_skb failed\n");
+ skb = actcapi_mkhdr(card, &m, 4, 0xff, 0x00);
+ if (!skb)
return -ENOMEM;
- }
+
m->msg.manufacturer_req_err.manuf_msg = 0x03;
m->msg.manufacturer_req_err.controller = 0;
- ACTCAPI_QUEUE_TX;
+ actcapi_queue_tx(card, skb);
return 0;
}
@@ -281,17 +284,16 @@ actcapi_manufacturer_req_msn(act2000_card *card)
len = strlen(p->msn);
for (i = 0; i < 2; i++) {
- ACTCAPI_MKHDR(6 + len, 0xff, 0x00);
- if (!skb) {
- printk(KERN_WARNING "actcapi: alloc_skb failed\n");
+ skb = actcapi_mkhdr(card, &m, 6 + len, 0xff, 0x00);
+ if (!skb)
return -ENOMEM;
- }
+
m->msg.manufacturer_req_msn.manuf_msg = 0x13 + i;
m->msg.manufacturer_req_msn.controller = 0;
m->msg.manufacturer_req_msn.msnmap.eaz = p->eaz;
m->msg.manufacturer_req_msn.msnmap.len = len;
memcpy(m->msg.manufacturer_req_msn.msnmap.msn, p->msn, len);
- ACTCAPI_QUEUE_TX;
+ actcapi_queue_tx(card, skb);
}
p = p->next;
}
@@ -304,8 +306,10 @@ actcapi_select_b2_protocol_req(act2000_card *card, act2000_chan *chan)
actcapi_msg *m;
struct sk_buff *skb;
- ACTCAPI_MKHDR(10, 0x40, 0x00);
- ACTCAPI_CHKSKB;
+ skb = actcapi_mkhdr(card, &m, 10, 0x40, 0x00);
+ if (!skb)
+ return;
+
m->msg.select_b2_protocol_req.plci = chan->plci;
memset(&m->msg.select_b2_protocol_req.dlpd, 0,
sizeof(m->msg.select_b2_protocol_req.dlpd));
@@ -330,7 +334,7 @@ actcapi_select_b2_protocol_req(act2000_card *card, act2000_chan *chan)
m->msg.select_b2_protocol_req.dlpd.modulo = 8;
break;
}
- ACTCAPI_QUEUE_TX;
+ actcapi_queue_tx(card, skb);
}
static void
@@ -339,8 +343,10 @@ actcapi_select_b3_protocol_req(act2000_card *card, act2000_chan *chan)
actcapi_msg *m;
struct sk_buff *skb;
- ACTCAPI_MKHDR(17, 0x80, 0x00);
- ACTCAPI_CHKSKB;
+ skb = actcapi_mkhdr(card, &m, 17, 0x80, 0x00);
+ if (!skb)
+ return;
+
m->msg.select_b3_protocol_req.plci = chan->plci;
memset(&m->msg.select_b3_protocol_req.ncpd, 0,
sizeof(m->msg.select_b3_protocol_req.ncpd));
@@ -351,7 +357,7 @@ actcapi_select_b3_protocol_req(act2000_card *card, act2000_chan *chan)
m->msg.select_b3_protocol_req.ncpd.modulo = 8;
break;
}
- ACTCAPI_QUEUE_TX;
+ actcapi_queue_tx(card, skb);
}
static void
@@ -360,10 +366,12 @@ actcapi_listen_b3_req(act2000_card *card, act2000_chan *chan)
actcapi_msg *m;
struct sk_buff *skb;
- ACTCAPI_MKHDR(2, 0x81, 0x00);
- ACTCAPI_CHKSKB;
+ skb = actcapi_mkhdr(card, &m, 2, 0x81, 0x00);
+ if (!skb)
+ return;
+
m->msg.listen_b3_req.plci = chan->plci;
- ACTCAPI_QUEUE_TX;
+ actcapi_queue_tx(card, skb);
}
static void
@@ -372,11 +380,13 @@ actcapi_disconnect_req(act2000_card *card, act2000_chan *chan)
actcapi_msg *m;
struct sk_buff *skb;
- ACTCAPI_MKHDR(3, 0x04, 0x00);
- ACTCAPI_CHKSKB;
+ skb = actcapi_mkhdr(card, &m, 3, 0x04, 0x00);
+ if (!skb)
+ return;
+
m->msg.disconnect_req.plci = chan->plci;
m->msg.disconnect_req.cause = 0;
- ACTCAPI_QUEUE_TX;
+ actcapi_queue_tx(card, skb);
}
void
@@ -385,15 +395,17 @@ actcapi_disconnect_b3_req(act2000_card *card, act2000_chan *chan)
actcapi_msg *m;
struct sk_buff *skb;
- ACTCAPI_MKHDR(17, 0x84, 0x00);
- ACTCAPI_CHKSKB;
+ skb = actcapi_mkhdr(card, &m, 17, 0x84, 0x00);
+ if (!skb)
+ return;
+
m->msg.disconnect_b3_req.ncci = chan->ncci;
memset(&m->msg.disconnect_b3_req.ncpi, 0,
sizeof(m->msg.disconnect_b3_req.ncpi));
m->msg.disconnect_b3_req.ncpi.len = 13;
m->msg.disconnect_b3_req.ncpi.modulo = 8;
chan->fsm_state = ACT2000_STATE_BHWAIT;
- ACTCAPI_QUEUE_TX;
+ actcapi_queue_tx(card, skb);
}
void
@@ -402,8 +414,10 @@ actcapi_connect_resp(act2000_card *card, act2000_chan *chan, __u8 cause)
actcapi_msg *m;
struct sk_buff *skb;
- ACTCAPI_MKHDR(3, 0x02, 0x03);
- ACTCAPI_CHKSKB;
+ skb = actcapi_mkhdr(card, &m, 3, 0x02, 0x03);
+ if (!skb)
+ return;
+
m->msg.connect_resp.plci = chan->plci;
m->msg.connect_resp.rejectcause = cause;
if (cause) {
@@ -411,7 +425,7 @@ actcapi_connect_resp(act2000_card *card, act2000_chan *chan, __u8 cause)
chan->plci = 0x8000;
} else
chan->fsm_state = ACT2000_STATE_IWAIT;
- ACTCAPI_QUEUE_TX;
+ actcapi_queue_tx(card, skb);
}
static void
@@ -420,12 +434,14 @@ actcapi_connect_active_resp(act2000_card *card, act2000_chan *chan)
actcapi_msg *m;
struct sk_buff *skb;
- ACTCAPI_MKHDR(2, 0x03, 0x03);
- ACTCAPI_CHKSKB;
+ skb = actcapi_mkhdr(card, &m, 2, 0x03, 0x03);
+ if (!skb)
+ return;
+
m->msg.connect_resp.plci = chan->plci;
if (chan->fsm_state == ACT2000_STATE_IWAIT)
chan->fsm_state = ACT2000_STATE_IBWAIT;
- ACTCAPI_QUEUE_TX;
+ actcapi_queue_tx(card, skb);
}
static void
@@ -434,8 +450,10 @@ actcapi_connect_b3_resp(act2000_card *card, act2000_chan *chan, __u8 rejectcause
actcapi_msg *m;
struct sk_buff *skb;
- ACTCAPI_MKHDR((rejectcause ? 3 : 17), 0x82, 0x03);
- ACTCAPI_CHKSKB;
+ skb = actcapi_mkhdr(card, &m, rejectcause ? 3 : 17, 0x82, 0x03);
+ if (!skb)
+ return;
+
m->msg.connect_b3_resp.ncci = chan->ncci;
m->msg.connect_b3_resp.rejectcause = rejectcause;
if (!rejectcause) {
@@ -445,7 +463,7 @@ actcapi_connect_b3_resp(act2000_card *card, act2000_chan *chan, __u8 rejectcause
m->msg.connect_b3_resp.ncpi.modulo = 8;
chan->fsm_state = ACT2000_STATE_BWAIT;
}
- ACTCAPI_QUEUE_TX;
+ actcapi_queue_tx(card, skb);
}
static void
@@ -454,11 +472,13 @@ actcapi_connect_b3_active_resp(act2000_card *card, act2000_chan *chan)
actcapi_msg *m;
struct sk_buff *skb;
- ACTCAPI_MKHDR(2, 0x83, 0x03);
- ACTCAPI_CHKSKB;
+ skb = actcapi_mkhdr(card, &m, 2, 0x83, 0x03);
+ if (!skb)
+ return;
+
m->msg.connect_b3_active_resp.ncci = chan->ncci;
chan->fsm_state = ACT2000_STATE_ACTIVE;
- ACTCAPI_QUEUE_TX;
+ actcapi_queue_tx(card, skb);
}
static void
@@ -467,10 +487,12 @@ actcapi_info_resp(act2000_card *card, act2000_chan *chan)
actcapi_msg *m;
struct sk_buff *skb;
- ACTCAPI_MKHDR(2, 0x07, 0x03);
- ACTCAPI_CHKSKB;
+ skb = actcapi_mkhdr(card, &m, 2, 0x07, 0x03);
+ if (!skb)
+ return;
+
m->msg.info_resp.plci = chan->plci;
- ACTCAPI_QUEUE_TX;
+ actcapi_queue_tx(card, skb);
}
static void
@@ -479,12 +501,14 @@ actcapi_disconnect_b3_resp(act2000_card *card, act2000_chan *chan)
actcapi_msg *m;
struct sk_buff *skb;
- ACTCAPI_MKHDR(2, 0x84, 0x03);
- ACTCAPI_CHKSKB;
+ skb = actcapi_mkhdr(card, &m, 2, 0x84, 0x03);
+ if (!skb)
+ return;
+
m->msg.disconnect_b3_resp.ncci = chan->ncci;
chan->ncci = 0x8000;
chan->queued = 0;
- ACTCAPI_QUEUE_TX;
+ actcapi_queue_tx(card, skb);
}
static void
@@ -493,11 +517,13 @@ actcapi_disconnect_resp(act2000_card *card, act2000_chan *chan)
actcapi_msg *m;
struct sk_buff *skb;
- ACTCAPI_MKHDR(2, 0x04, 0x03);
- ACTCAPI_CHKSKB;
+ skb = actcapi_mkhdr(card, &m, 2, 0x04, 0x03);
+ if (!skb)
+ return;
+
m->msg.disconnect_resp.plci = chan->plci;
chan->plci = 0x8000;
- ACTCAPI_QUEUE_TX;
+ actcapi_queue_tx(card, skb);
}
static int
@@ -575,7 +601,7 @@ actcapi_data_b3_ind(act2000_card *card, struct sk_buff *skb) {
msg->hdr.msgnum = actcapi_nextsmsg(card);
msg->msg.data_b3_resp.ncci = ncci;
msg->msg.data_b3_resp.blocknr = blocknr;
- ACTCAPI_QUEUE_TX;
+ actcapi_queue_tx(card, skb);
return 1;
}
^ permalink raw reply related
* Re: [PATCH next 5/6] bonding: Allow userspace to set macaddr on bonding-dev
From: Mahesh Bandewar @ 2015-02-07 19:15 UTC (permalink / raw)
To: Jay Vosburgh
Cc: Andy Gospodarek, Veaceslav Falico, Nikolay Aleksandrov,
David Miller, netdev, Eric Dumazet, Maciej Żenczykowski
In-Reply-To: <7179.1423291742@famine>
On Fri, Feb 6, 2015 at 10:49 PM, Jay Vosburgh
<jay.vosburgh@canonical.com> wrote:
> Mahesh Bandewar <maheshb@google.com> wrote:
>
>>On Fri, Feb 6, 2015 at 5:54 PM, Jay Vosburgh <jay.vosburgh@canonical.com> wrote:
>>> Mahesh Bandewar <maheshb@google.com> wrote:
>>>
>>>>On Fri, Feb 6, 2015 at 5:20 PM, Jay Vosburgh <jay.vosburgh@canonical.com> wrote:
>>>>> Mahesh Bandewar <maheshb@google.com> wrote:
>>>>>
>>>>>>This patch allows user-space to set the mac-address on the bonding device.
>>>>>>This mac-address can not be NULL or a Multicast. If the mac-address is set
>>>>>>from user-space; kernel will honor it and will not overwrite it. In the
>>>>>>absense (value from user space); the logic will default to using the
>>>>>>masters' mac as the mac address for the bonding device.
>>>>>>
>>>>>>It can be set using example code below -
>>>>>>
>>>>>> # modprobe bonding mode=4
>>>>>> # sys_mac_addr=$(printf '%02x:%02x:%02x:%02x:%02x:%02x' \
>>>>>> $(( (RANDOM & 0xFE) | 0x02 )) \
>>>>>> $(( RANDOM & 0xFF )) \
>>>>>> $(( RANDOM & 0xFF )) \
>>>>>> $(( RANDOM & 0xFF )) \
>>>>>> $(( RANDOM & 0xFF )) \
>>>>>> $(( RANDOM & 0xFF )))
>>>>>> # echo $sys_mac_addr > /sys/class/net/bond0/bonding/ad_actor_system_mac_address
>>>>>> # echo +eth1 > /sys/class/net/bond0/bonding/slaves
>>>>>> ...
>>>>>> # ip link set bond0 up
>>>>>
>>>>> How is this patch functionally different from setting the
>>>>> bonding master's MAC address to a particular value prior to adding any
>>>>> slaves?
>>>>>
>>>>
>>>>Maciej is correct but I think I was bit ambiguous about it in the
>>>>commit message which might have made you think this way. I'll reword
>>>>the commit message.
>>>
>>> Thanks; presumably there is some administrative reason for this.
>>>
>>The idea is that in an AD system the actor-partner communication is a
>>business between them two only and no one in the L2 domain should
>>care. These enhancements should obscure that should anyone try to
>>sniff it. Probably I assumed too much and should add this idea behind
>>the implementation in the commit log.
>
> Well, the LACPDUs can always be sniffed on ingress to the
> destination linux system with something like tcpdump, but they don't
> propagate, so I'm not sure how a third party would ever see them. The
> actor_system value isn't really a secret, either, in the sense of
> needing to be kept hidden (your patch even adds it to
> /proc/net/bonding/*).
>
> Has there been some kind of issue with this?
>
> In any event, the requirements for the actor_system in the
> standard are pretty much that it's a globally unique MAC address, so
> there's no real issue with permitting it to be set from that
> perspective.
>
As Maciej has explained there is that possibility and with these
enhancements, it not hidden but just obscured so that the device in
the same L2 domain can not pinpoint these DUs to a specific neighbor.
>>> Also, for patches 4, 5 and 6, I believe current practice is to
>>> provide a netlink / iproute2 facility for options.
>>>
>>OK. I'll add them (netlink enhancements) in a separate set of patch(s).
>
> Also, I neglected to mention that the new options should be
> added to Documentation/networking/bonding.txt.
>
Yes, I'll update the bonding.txt file in the next patch revision.
>>>>>>Signed-off-by: Mahesh Bandewar <maheshb@google.com>
>>>>>>---
>>>>>> drivers/net/bonding/bond_3ad.c | 7 ++++++-
>>>>>> drivers/net/bonding/bond_main.c | 1 +
>>>>>> drivers/net/bonding/bond_options.c | 29 +++++++++++++++++++++++++++++
>>>>>> drivers/net/bonding/bond_procfs.c | 6 ++++++
>>>>>> drivers/net/bonding/bond_sysfs.c | 16 ++++++++++++++++
>>>>>> include/net/bond_options.h | 1 +
>>>>>> include/net/bonding.h | 1 +
>>>>>> 7 files changed, 60 insertions(+), 1 deletion(-)
>>>>>>
>>>>>>diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
>>>>>>index 1177f96194dd..373d3db3809f 100644
>>>>>>--- a/drivers/net/bonding/bond_3ad.c
>>>>>>+++ b/drivers/net/bonding/bond_3ad.c
>>>>>>@@ -1914,7 +1914,12 @@ void bond_3ad_initialize(struct bonding *bond, u16 tick_resolution)
>>>>>>
>>>>>> BOND_AD_INFO(bond).system.sys_priority =
>>>>>> bond->params.ad_actor_sysprio;
>>>>>>- BOND_AD_INFO(bond).system.sys_mac_addr = *((struct mac_addr *)bond->dev->dev_addr);
>>>>>>+ if (is_zero_ether_addr(bond->params.ad_actor_sys_macaddr))
>>>>>>+ BOND_AD_INFO(bond).system.sys_mac_addr =
>>>>>>+ *((struct mac_addr *)bond->dev->dev_addr);
>>>>>>+ else
>>>>>>+ BOND_AD_INFO(bond).system.sys_mac_addr =
>>>>>>+ *((struct mac_addr *)bond->params.ad_actor_sys_macaddr);
>>>>>>
>>>>>> /* initialize how many times this module is called in one
>>>>>> * second (should be about every 100ms)
>>>>>>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>>>>>>index 561b2bde5aeb..1a6735ef2ea7 100644
>>>>>>--- a/drivers/net/bonding/bond_main.c
>>>>>>+++ b/drivers/net/bonding/bond_main.c
>>>>>>@@ -4440,6 +4440,7 @@ static int bond_check_params(struct bond_params *params)
>>>>>> params->packets_per_slave = packets_per_slave;
>>>>>> params->tlb_dynamic_lb = 1; /* Default value */
>>>>>> params->ad_actor_sysprio = ad_actor_sysprio;
>>>>>>+ eth_zero_addr(params->ad_actor_sys_macaddr);
>>>>>> if (packets_per_slave > 0) {
>>>>>> params->reciprocal_packets_per_slave =
>>>>>> reciprocal_value(packets_per_slave);
>>>>>>diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
>>>>>>index d8f6760143ae..330d48b6a1e6 100644
>>>>>>--- a/drivers/net/bonding/bond_options.c
>>>>>>+++ b/drivers/net/bonding/bond_options.c
>>>>>>@@ -72,6 +72,8 @@ static int bond_option_tlb_dynamic_lb_set(struct bonding *bond,
>>>>>> const struct bond_opt_value *newval);
>>>>>> static int bond_option_ad_actor_sysprio_set(struct bonding *bond,
>>>>>> const struct bond_opt_value *newval);
>>>>>>+static int bond_option_ad_actor_sys_macaddr_set(struct bonding *bond,
>>>>>>+ const struct bond_opt_value *newval);
>>>>>>
>>>>>>
>>>>>> static const struct bond_opt_value bond_mode_tbl[] = {
>>>>>>@@ -396,6 +398,13 @@ static const struct bond_option bond_opts[BOND_OPT_LAST] = {
>>>>>> .values = bond_ad_actor_sysprio_tbl,
>>>>>> .set = bond_option_ad_actor_sysprio_set,
>>>>>> },
>>>>>>+ [BOND_OPT_AD_ACTOR_SYS_MACADDR] = {
>>>>>>+ .id = BOND_OPT_AD_ACTOR_SYS_MACADDR,
>>>>>>+ .name = "ad_actor_system_mac_address",
>>>>>>+ .unsuppmodes = BOND_MODE_ALL_EX(BIT(BOND_MODE_8023AD)),
>>>>>>+ .flags = BOND_OPTFLAG_RAWVAL | BOND_OPTFLAG_IFDOWN,
>>>>>>+ .set = bond_option_ad_actor_sys_macaddr_set,
>>>>>>+ },
>>>>>> };
>>>>>>
>>>>>> /* Searches for an option by name */
>>>>>>@@ -1376,3 +1385,23 @@ static int bond_option_ad_actor_sysprio_set(struct bonding *bond,
>>>>>> bond->params.ad_actor_sysprio = newval->value;
>>>>>> return 0;
>>>>>> }
>>>>>>+
>>>>>>+static int bond_option_ad_actor_sys_macaddr_set(struct bonding *bond,
>>>>>>+ const struct bond_opt_value *newval)
>>>>>>+{
>>>>>>+ u8 macaddr[ETH_ALEN];
>>>>>>+ int i;
>>>>>>+
>>>>>>+ i = sscanf(newval->string, "%hhx:%hhx:%hhx:%hhx:%hhx:%hhx",
>>>>>>+ &macaddr[0], &macaddr[1], &macaddr[2],
>>>>>>+ &macaddr[3], &macaddr[4], &macaddr[5]);
>>>>>>+
>>>>>>+ if (i != ETH_ALEN || !is_valid_ether_addr(macaddr)) {
>>>>>>+ netdev_err(bond->dev, "Invalid MAC address.\n");
>>>>>>+ return -EINVAL;
>>>>>>+ }
>>>>>>+
>>>>>>+ ether_addr_copy(bond->params.ad_actor_sys_macaddr, macaddr);
>
> I think this operation should probably fail if the bond has any
> slaves, as the new value will not actually propagate out to what's being
> used in LACPDUs in that case.
>
> Looking at the other two new option patches, I think this
> comment also applies to them.
>
You can do this only when the the bond is down.
> -J
>
>>>>>>+ return 0;
>>>>>>+}
>>>>>>diff --git a/drivers/net/bonding/bond_procfs.c b/drivers/net/bonding/bond_procfs.c
>>>>>>index 9e33c48886ef..81452ced852f 100644
>>>>>>--- a/drivers/net/bonding/bond_procfs.c
>>>>>>+++ b/drivers/net/bonding/bond_procfs.c
>>>>>>@@ -136,6 +136,8 @@ static void bond_info_show_master(struct seq_file *seq)
>>>>>> optval->string);
>>>>>> seq_printf(seq, "System priority: %d\n",
>>>>>> BOND_AD_INFO(bond).system.sys_priority);
>>>>>>+ seq_printf(seq, "System MAC address: %pM\n",
>>>>>>+ &BOND_AD_INFO(bond).system.sys_mac_addr);
>>>>>>
>>>>>> if (__bond_3ad_get_active_agg_info(bond, &ad_info)) {
>>>>>> seq_printf(seq, "bond %s has no active aggregator\n",
>>>>>>@@ -198,6 +200,8 @@ static void bond_info_show_slave(struct seq_file *seq,
>>>>>> seq_puts(seq, "details actor lacp pdu:\n");
>>>>>> seq_printf(seq, " system priority: %d\n",
>>>>>> port->actor_system_priority);
>>>>>>+ seq_printf(seq, " system mac address: %pM\n",
>>>>>>+ &port->actor_system);
>>>>>> seq_printf(seq, " port key: %d\n",
>>>>>> port->actor_oper_port_key);
>>>>>> seq_printf(seq, " port priority: %d\n",
>>>>>>@@ -210,6 +214,8 @@ static void bond_info_show_slave(struct seq_file *seq,
>>>>>> seq_puts(seq, "details partner lacp pdu:\n");
>>>>>> seq_printf(seq, " system priority: %d\n",
>>>>>> port->partner_oper.system_priority);
>>>>>>+ seq_printf(seq, " system mac address: %pM\n",
>>>>>>+ &port->partner_oper.system);
>>>>>> seq_printf(seq, " oper key: %d\n",
>>>>>> port->partner_oper.key);
>>>>>> seq_printf(seq, " port priority: %d\n",
>>>>>>diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
>>>>>>index 4350aa06f867..91713d0b6685 100644
>>>>>>--- a/drivers/net/bonding/bond_sysfs.c
>>>>>>+++ b/drivers/net/bonding/bond_sysfs.c
>>>>>>@@ -706,6 +706,21 @@ static ssize_t bonding_show_ad_actor_sysprio(struct device *d,
>>>>>> static DEVICE_ATTR(ad_actor_system_priority, S_IRUGO | S_IWUSR,
>>>>>> bonding_show_ad_actor_sysprio, bonding_sysfs_store_option);
>>>>>>
>>>>>>+static ssize_t bonding_show_ad_actor_sys_macaddr(struct device *d,
>>>>>>+ struct device_attribute *attr,
>>>>>>+ char *buf)
>>>>>>+{
>>>>>>+ struct bonding *bond = to_bond(d);
>>>>>>+
>>>>>>+ if (BOND_MODE(bond) == BOND_MODE_8023AD)
>>>>>>+ return sprintf(buf, "%pM\n", bond->params.ad_actor_sys_macaddr);
>>>>>>+
>>>>>>+ return 0;
>>>>>>+}
>>>>>>+static DEVICE_ATTR(ad_actor_system_mac_address, S_IRUGO | S_IWUSR,
>>>>>>+ bonding_show_ad_actor_sys_macaddr,
>>>>>>+ bonding_sysfs_store_option);
>>>>>>+
>>>>>> static struct attribute *per_bond_attrs[] = {
>>>>>> &dev_attr_slaves.attr,
>>>>>> &dev_attr_mode.attr,
>>>>>>@@ -740,6 +755,7 @@ static struct attribute *per_bond_attrs[] = {
>>>>>> &dev_attr_packets_per_slave.attr,
>>>>>> &dev_attr_tlb_dynamic_lb.attr,
>>>>>> &dev_attr_ad_actor_system_priority.attr,
>>>>>>+ &dev_attr_ad_actor_system_mac_address.attr,
>>>>>> NULL,
>>>>>> };
>>>>>>
>>>>>>diff --git a/include/net/bond_options.h b/include/net/bond_options.h
>>>>>>index c2af1db37354..993ef73cd050 100644
>>>>>>--- a/include/net/bond_options.h
>>>>>>+++ b/include/net/bond_options.h
>>>>>>@@ -64,6 +64,7 @@ enum {
>>>>>> BOND_OPT_SLAVES,
>>>>>> BOND_OPT_TLB_DYNAMIC_LB,
>>>>>> BOND_OPT_AD_ACTOR_SYSPRIO,
>>>>>>+ BOND_OPT_AD_ACTOR_SYS_MACADDR,
>>>>>> BOND_OPT_LAST
>>>>>> };
>>>>>>
>>>>>>diff --git a/include/net/bonding.h b/include/net/bonding.h
>>>>>>index 7a5c79fcf866..cd2092e6bc71 100644
>>>>>>--- a/include/net/bonding.h
>>>>>>+++ b/include/net/bonding.h
>>>>>>@@ -144,6 +144,7 @@ struct bond_params {
>>>>>> int tlb_dynamic_lb;
>>>>>> struct reciprocal_value reciprocal_packets_per_slave;
>>>>>> u16 ad_actor_sysprio;
>>>>>>+ u8 ad_actor_sys_macaddr[ETH_ALEN];
>>>>>> };
>>>>>>
>>>>>> struct bond_parm_tbl {
>>>>>>--
>>>>>>2.2.0.rc0.207.ga3a616c
>
> ---
> -Jay Vosburgh, jay.vosburgh@canonical.com
^ permalink raw reply
* Re: [PATCH 7/7] drivers: isdn: act2000: capi.c: add macro \ and fix brace
From: Julia Lawall @ 2015-02-07 19:15 UTC (permalink / raw)
To: Bas Peters
Cc: isdn, julia.lawall, davem, netdev, linux-kernel, sergei.shtylyov
In-Reply-To: <1423335929-24926-8-git-send-email-baspeters93@gmail.com>
On Sat, 7 Feb 2015, Bas Peters wrote:
> This patch adds the \ that was accidentally deleted in patch 2. It also adds a brace after the else statement, which is required due to the fact that the if statement has braces.
You should fix the patch that was incorrect, rather than submitting a
patch that does something incorrect and then another patch to fix it. The
kernel should compile after every patch.
julia
>
> ---
> drivers/isdn/act2000/capi.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/isdn/act2000/capi.c b/drivers/isdn/act2000/capi.c
> index 5d677e6..0043b3c 100644
> --- a/drivers/isdn/act2000/capi.c
> +++ b/drivers/isdn/act2000/capi.c
> @@ -113,8 +113,9 @@ actcapi_chkhdr(act2000_card *card, actcapi_msghdr *hdr)
> m->hdr.cmd.cmd = c; \
> m->hdr.cmd.subcmd = s; \
> m->hdr.msgnum = actcapi_nextsmsg(card); \
> - } else
> + } else { \
> m = NULL; \
> + } \
> }
>
> #define ACTCAPI_CHKSKB if (!skb) { \
> --
> 2.1.0
>
>
^ permalink raw reply
* Re: [PATCH 7/7] drivers: isdn: act2000: capi.c: add macro \ and fix brace
From: Sergei Shtylyov @ 2015-02-07 19:12 UTC (permalink / raw)
To: Bas Peters, isdn; +Cc: julia.lawall, davem, netdev, linux-kernel
In-Reply-To: <1423335929-24926-8-git-send-email-baspeters93@gmail.com>
On 02/07/2015 10:05 PM, Bas Peters wrote:
> This patch adds the \ that was accidentally deleted in patch 2. It also adds a brace after the else statement, which is required due to the fact that the if statement has braces.
This won't do, fix up the patch #2 please. And please wrap your change log
at 80 columns or less. And you forgot to sign off anyway. :-)
[...]
WBR, Sergei
^ permalink raw reply
* Re: [PATCH 2/7] drivers: isdn: act2000: capi.c: fix checkpatch errors
From: Sergei Shtylyov @ 2015-02-07 19:09 UTC (permalink / raw)
To: Bas Peters, isdn; +Cc: julia.lawall, davem, netdev, linux-kernel
In-Reply-To: <1423335929-24926-3-git-send-email-baspeters93@gmail.com>
On 02/07/2015 10:05 PM, Bas Peters wrote:
> This patch fixes the following checkpatch errors:
> 1. trailing statement
> 1. assignment of variable in if condition
> 1. incorrectly placed brace after function definition
> Signed-off-by: Bas Peters <baspeters93@gmail.com>
> ---
> drivers/isdn/act2000/capi.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
> diff --git a/drivers/isdn/act2000/capi.c b/drivers/isdn/act2000/capi.c
> index 3f66ca2..5d677e6 100644
> --- a/drivers/isdn/act2000/capi.c
> +++ b/drivers/isdn/act2000/capi.c
> @@ -113,7 +113,8 @@ actcapi_chkhdr(act2000_card *card, actcapi_msghdr *hdr)
> m->hdr.cmd.cmd = c; \
> m->hdr.cmd.subcmd = s; \
> m->hdr.msgnum = actcapi_nextsmsg(card); \
> - } else m = NULL; \
> + } else
Backslash is still missing here.
> + m = NULL; \
> }
>
> #define ACTCAPI_CHKSKB if (!skb) { \
[...]
WBR, Sergei
^ permalink raw reply
* [PATCH 7/7] drivers: isdn: act2000: capi.c: add macro \ and fix brace
From: Bas Peters @ 2015-02-07 19:05 UTC (permalink / raw)
To: isdn; +Cc: julia.lawall, davem, netdev, linux-kernel, sergei.shtylyov,
Bas Peters
In-Reply-To: <1423335929-24926-1-git-send-email-baspeters93@gmail.com>
This patch adds the \ that was accidentally deleted in patch 2. It also adds a brace after the else statement, which is required due to the fact that the if statement has braces.
---
drivers/isdn/act2000/capi.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/isdn/act2000/capi.c b/drivers/isdn/act2000/capi.c
index 5d677e6..0043b3c 100644
--- a/drivers/isdn/act2000/capi.c
+++ b/drivers/isdn/act2000/capi.c
@@ -113,8 +113,9 @@ actcapi_chkhdr(act2000_card *card, actcapi_msghdr *hdr)
m->hdr.cmd.cmd = c; \
m->hdr.cmd.subcmd = s; \
m->hdr.msgnum = actcapi_nextsmsg(card); \
- } else
+ } else { \
m = NULL; \
+ } \
}
#define ACTCAPI_CHKSKB if (!skb) { \
--
2.1.0
^ permalink raw reply related
* [PATCH 6/7] drivers: isdn: act2000: fix wrongly positioned brace.
From: Bas Peters @ 2015-02-07 19:05 UTC (permalink / raw)
To: isdn; +Cc: julia.lawall, davem, netdev, linux-kernel, sergei.shtylyov,
Bas Peters
In-Reply-To: <1423335929-24926-1-git-send-email-baspeters93@gmail.com>
Trivial, but why not? :)
Signed-off-by: Bas Peters <baspeters93@gmail.com>
---
drivers/isdn/act2000/module.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/isdn/act2000/module.c b/drivers/isdn/act2000/module.c
index 889ffcb..9ba98ce 100644
--- a/drivers/isdn/act2000/module.c
+++ b/drivers/isdn/act2000/module.c
@@ -19,8 +19,7 @@
#include <linux/slab.h>
#include <linux/init.h>
-static unsigned short act2000_isa_ports[] =
-{
+static unsigned short act2000_isa_ports[] = {
0x0200, 0x0240, 0x0280, 0x02c0, 0x0300, 0x0340, 0x0380,
0xcfe0, 0xcfa0, 0xcf60, 0xcf20, 0xcee0, 0xcea0, 0xce60,
};
--
2.1.0
^ permalink raw reply related
* [PATCH 5/7] drivers: isdn: act2000: module.c: remove parenthesres around return values.
From: Bas Peters @ 2015-02-07 19:05 UTC (permalink / raw)
To: isdn; +Cc: julia.lawall, davem, netdev, linux-kernel, sergei.shtylyov,
Bas Peters
In-Reply-To: <1423335929-24926-1-git-send-email-baspeters93@gmail.com>
return is not a function, therefore parentheses are not needed.
---
drivers/isdn/act2000/module.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/isdn/act2000/module.c b/drivers/isdn/act2000/module.c
index 9359b36..889ffcb 100644
--- a/drivers/isdn/act2000/module.c
+++ b/drivers/isdn/act2000/module.c
@@ -111,7 +111,7 @@ act2000_find_eaz(act2000_card *card, char eaz)
while (p) {
if (p->eaz == eaz)
- return (p->msn);
+ return p->msn;
p = p->next;
}
return ("\0");
@@ -293,7 +293,7 @@ act2000_command(act2000_card *card, isdn_ctrl *c)
if (ret)
return ret;
if (card->flags & ACT2000_FLAGS_RUNNING)
- return (actcapi_manufacturer_req_msn(card));
+ return actcapi_manufacturer_req_msn(card);
return 0;
case ACT2000_IOCTL_ADDCARD:
if (copy_from_user(&cdef, arg,
@@ -520,7 +520,7 @@ if_command(isdn_ctrl *c)
act2000_card *card = act2000_findcard(c->driver);
if (card)
- return (act2000_command(card, c));
+ return act2000_command(card, c);
printk(KERN_ERR
"act2000: if_command %d called with invalid driverId %d!\n",
c->command, c->driver);
@@ -535,7 +535,7 @@ if_writecmd(const u_char __user *buf, int len, int id, int channel)
if (card) {
if (!(card->flags & ACT2000_FLAGS_RUNNING))
return -ENODEV;
- return (len);
+ return len;
}
printk(KERN_ERR
"act2000: if_writecmd called with invalid driverId!\n");
@@ -550,7 +550,7 @@ if_readstatus(u_char __user *buf, int len, int id, int channel)
if (card) {
if (!(card->flags & ACT2000_FLAGS_RUNNING))
return -ENODEV;
- return (act2000_readstatus(buf, len, card));
+ return act2000_readstatus(buf, len, card);
}
printk(KERN_ERR
"act2000: if_readstatus called with invalid driverId!\n");
@@ -565,7 +565,7 @@ if_sendbuf(int id, int channel, int ack, struct sk_buff *skb)
if (card) {
if (!(card->flags & ACT2000_FLAGS_RUNNING))
return -ENODEV;
- return (act2000_sendbuf(card, channel, ack, skb));
+ return act2000_sendbuf(card, channel, ack, skb);
}
printk(KERN_ERR
"act2000: if_sendbuf called with invalid driverId!\n");
--
2.1.0
^ permalink raw reply related
* [PATCH 4/7] drivers: isdn: act2000: module.c: remove NULL-initialization of static variable.
From: Bas Peters @ 2015-02-07 19:05 UTC (permalink / raw)
To: isdn; +Cc: julia.lawall, davem, netdev, linux-kernel, sergei.shtylyov,
Bas Peters
In-Reply-To: <1423335929-24926-1-git-send-email-baspeters93@gmail.com>
GCC takes care of this for us, thus it is not needed and theoretically
only hoggs memory, allbeit only a bit.
---
drivers/isdn/act2000/module.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/isdn/act2000/module.c b/drivers/isdn/act2000/module.c
index 352916a..9359b36 100644
--- a/drivers/isdn/act2000/module.c
+++ b/drivers/isdn/act2000/module.c
@@ -28,7 +28,7 @@ static unsigned short act2000_isa_ports[] =
static act2000_card *cards = (act2000_card *) NULL;
/* Parameters to be set by insmod */
-static int act_bus = 0;
+static int act_bus;
static int act_port = -1; /* -1 = Autoprobe */
static int act_irq = -1;
static char *act_id = "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0";
--
2.1.0
^ permalink raw reply related
* [PATCH 3/7] drivers: isdn: act2000: remove assignments of variables in if conditions
From: Bas Peters @ 2015-02-07 19:05 UTC (permalink / raw)
To: isdn; +Cc: julia.lawall, davem, netdev, linux-kernel, sergei.shtylyov,
Bas Peters
In-Reply-To: <1423335929-24926-1-git-send-email-baspeters93@gmail.com>
This patch removes all assignments of if conditions, which is not in
accordance with the CodingStyle.
---
drivers/isdn/act2000/module.c | 30 ++++++++++++++++++++----------
1 file changed, 20 insertions(+), 10 deletions(-)
diff --git a/drivers/isdn/act2000/module.c b/drivers/isdn/act2000/module.c
index c3a1b06..352916a 100644
--- a/drivers/isdn/act2000/module.c
+++ b/drivers/isdn/act2000/module.c
@@ -289,7 +289,8 @@ act2000_command(act2000_card *card, isdn_ctrl *c)
if (copy_from_user(tmp, arg,
sizeof(tmp)))
return -EFAULT;
- if ((ret = act2000_set_msn(card, tmp)))
+ ret = act2000_set_msn(card, tmp);
+ if (ret)
return ret;
if (card->flags & ACT2000_FLAGS_RUNNING)
return (actcapi_manufacturer_req_msn(card));
@@ -312,7 +313,8 @@ act2000_command(act2000_card *card, isdn_ctrl *c)
case ISDN_CMD_DIAL:
if (!(card->flags & ACT2000_FLAGS_RUNNING))
return -ENODEV;
- if (!(chan = find_channel(card, c->arg & 0x0f)))
+ chan = find_channel(card, c->arg & 0x0f);
+ if (!chan)
break;
spin_lock_irqsave(&card->lock, flags);
if (chan->fsm_state != ACT2000_STATE_NULL) {
@@ -341,7 +343,8 @@ act2000_command(act2000_card *card, isdn_ctrl *c)
case ISDN_CMD_ACCEPTD:
if (!(card->flags & ACT2000_FLAGS_RUNNING))
return -ENODEV;
- if (!(chan = find_channel(card, c->arg & 0x0f)))
+ chan = find_channel(card, c->arg & 0x0f);
+ if (!chan)
break;
if (chan->fsm_state == ACT2000_STATE_ICALL)
actcapi_select_b2_protocol_req(card, chan);
@@ -353,7 +356,8 @@ act2000_command(act2000_card *card, isdn_ctrl *c)
case ISDN_CMD_HANGUP:
if (!(card->flags & ACT2000_FLAGS_RUNNING))
return -ENODEV;
- if (!(chan = find_channel(card, c->arg & 0x0f)))
+ chan = find_channel(card, c->arg & 0x0f);
+ if (!chan)
break;
switch (chan->fsm_state) {
case ACT2000_STATE_ICALL:
@@ -368,7 +372,8 @@ act2000_command(act2000_card *card, isdn_ctrl *c)
case ISDN_CMD_SETEAZ:
if (!(card->flags & ACT2000_FLAGS_RUNNING))
return -ENODEV;
- if (!(chan = find_channel(card, c->arg & 0x0f)))
+ chan = find_channel(card, c->arg & 0x0f);
+ if (!chan)
break;
if (strlen(c->parm.num)) {
if (card->ptype == ISDN_PTYPE_EURO) {
@@ -388,7 +393,8 @@ act2000_command(act2000_card *card, isdn_ctrl *c)
case ISDN_CMD_CLREAZ:
if (!(card->flags & ACT2000_FLAGS_RUNNING))
return -ENODEV;
- if (!(chan = find_channel(card, c->arg & 0x0f)))
+ chan = find_channel(card, c->arg & 0x0f);
+ if (!chan)
break;
chan->eazmask = 0;
actcapi_listen_req(card);
@@ -396,7 +402,8 @@ act2000_command(act2000_card *card, isdn_ctrl *c)
case ISDN_CMD_SETL2:
if (!(card->flags & ACT2000_FLAGS_RUNNING))
return -ENODEV;
- if (!(chan = find_channel(card, c->arg & 0x0f)))
+ chan = find_channel(card, c->arg & 0x0f);
+ if (!chan)
break;
chan->l2prot = (c->arg >> 8);
return 0;
@@ -407,7 +414,8 @@ act2000_command(act2000_card *card, isdn_ctrl *c)
printk(KERN_WARNING "L3 protocol unknown\n");
return -1;
}
- if (!(chan = find_channel(card, c->arg & 0x0f)))
+ chan = find_channel(card, c->arg & 0x0f);
+ if (!chan)
break;
chan->l3prot = (c->arg >> 8);
return 0;
@@ -424,7 +432,8 @@ act2000_sendbuf(act2000_card *card, int channel, int ack, struct sk_buff *skb)
act2000_chan *chan;
actcapi_msg *msg;
- if (!(chan = find_channel(card, channel)))
+ chan = find_channel(card, channel);
+ if (!chan)
return -1;
if (chan->fsm_state != ACT2000_STATE_ACTIVE)
return -1;
@@ -573,7 +582,8 @@ act2000_alloccard(int bus, int port, int irq, char *id)
{
int i;
act2000_card *card;
- if (!(card = kzalloc(sizeof(act2000_card), GFP_KERNEL))) {
+ card = kzalloc(sizeof(act2000_card), GFP_KERNEL);
+ if (!card) {
printk(KERN_WARNING
"act2000: (%s) Could not allocate card-struct.\n", id);
return;
--
2.1.0
^ permalink raw reply related
* [PATCH 2/7] drivers: isdn: act2000: capi.c: fix checkpatch errors
From: Bas Peters @ 2015-02-07 19:05 UTC (permalink / raw)
To: isdn; +Cc: julia.lawall, davem, netdev, linux-kernel, sergei.shtylyov,
Bas Peters
In-Reply-To: <1423335929-24926-1-git-send-email-baspeters93@gmail.com>
This patch fixes the following checkpatch errors:
1. trailing statement
1. assignment of variable in if condition
1. incorrectly placed brace after function definition
Signed-off-by: Bas Peters <baspeters93@gmail.com>
---
drivers/isdn/act2000/capi.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/isdn/act2000/capi.c b/drivers/isdn/act2000/capi.c
index 3f66ca2..5d677e6 100644
--- a/drivers/isdn/act2000/capi.c
+++ b/drivers/isdn/act2000/capi.c
@@ -113,7 +113,8 @@ actcapi_chkhdr(act2000_card *card, actcapi_msghdr *hdr)
m->hdr.cmd.cmd = c; \
m->hdr.cmd.subcmd = s; \
m->hdr.msgnum = actcapi_nextsmsg(card); \
- } else m = NULL; \
+ } else
+ m = NULL; \
}
#define ACTCAPI_CHKSKB if (!skb) { \
@@ -563,7 +564,8 @@ actcapi_data_b3_ind(act2000_card *card, struct sk_buff *skb) {
blocknr = msg->msg.data_b3_ind.blocknr;
skb_pull(skb, 19);
card->interface.rcvcallb_skb(card->myid, chan, skb);
- if (!(skb = alloc_skb(11, GFP_ATOMIC))) {
+ skb = alloc_skb(11, GFP_ATOMIC);
+ if (!skb) {
printk(KERN_WARNING "actcapi: alloc_skb failed\n");
return 1;
}
@@ -990,7 +992,8 @@ actcapi_debug_dlpd(actcapi_dlpd *dlpd)
}
#ifdef DEBUG_DUMP_SKB
-static void dump_skb(struct sk_buff *skb) {
+static void dump_skb(struct sk_buff *skb)
+{
char tmp[80];
char *p = skb->data;
char *t = tmp;
--
2.1.0
^ permalink raw reply related
* [PATCH 1/7] drivers: isdn: act2000: act2000_isa.c: Fix checkpatch errors
From: Bas Peters @ 2015-02-07 19:05 UTC (permalink / raw)
To: isdn; +Cc: julia.lawall, davem, netdev, linux-kernel, sergei.shtylyov,
Bas Peters
In-Reply-To: <1423335929-24926-1-git-send-email-baspeters93@gmail.com>
This patch adresses various checkpatch errors:
3 assignments in if conditions
1 return value enclosed in parenthesis
Signed-off-by: Bas Peters <baspeters93@gmail.com>
---
drivers/isdn/act2000/act2000_isa.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/isdn/act2000/act2000_isa.c b/drivers/isdn/act2000/act2000_isa.c
index b5fad29..048507e 100644
--- a/drivers/isdn/act2000/act2000_isa.c
+++ b/drivers/isdn/act2000/act2000_isa.c
@@ -31,7 +31,8 @@ act2000_isa_reset(unsigned short portbase)
int serial = 0;
found = 0;
- if ((reg = inb(portbase + ISA_COR)) != 0xff) {
+ reg = inb(portbase + ISA_COR);
+ if (reg != 0xff) {
outb(reg | ISA_COR_RESET, portbase + ISA_COR);
mdelay(10);
outb(reg, portbase + ISA_COR);
@@ -303,7 +304,8 @@ act2000_isa_send(act2000_card *card)
while (1) {
spin_lock_irqsave(&card->lock, flags);
if (!(card->sbuf)) {
- if ((card->sbuf = skb_dequeue(&card->sndq))) {
+ card->sbuf = skb_dequeue(&card->sndq);
+ if (card->sbuf) {
card->ack_msg = card->sbuf->data;
msg = (actcapi_msg *)card->sbuf->data;
if ((msg->hdr.cmd.cmd == 0x86) &&
@@ -378,7 +380,8 @@ act2000_isa_getid(act2000_card *card)
printk(KERN_WARNING "act2000: Wrong Firmware-ID!\n");
return -EPROTO;
}
- if ((p = strchr(fid.revision, '\n')))
+ p = strchr(fid.revision, '\n');
+ if (p)
*p = '\0';
printk(KERN_INFO "act2000: Firmware-ID: %s\n", fid.revision);
if (card->flags & ACT2000_FLAGS_IVALID) {
@@ -439,5 +442,5 @@ act2000_isa_download(act2000_card *card, act2000_ddef __user *cb)
}
kfree(buf);
msleep_interruptible(500);
- return (act2000_isa_getid(card));
+ return act2000_isa_getid(card);
}
--
2.1.0
^ permalink raw reply related
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