* Re: [net-next 11/13] igb: Update PTP function names/variables and locations.
From: Joe Perches @ 2012-08-24 3:02 UTC (permalink / raw)
To: Richard Cochran
Cc: Vick, Matthew, Kirsher, Jeffrey T, davem@davemloft.net,
netdev@vger.kernel.org, gospo@redhat.com, sassmann@redhat.com
In-Reply-To: <20120823175335.GB2192@netboy.at.omicron.at>
On Thu, 2012-08-23 at 19:53 +0200, Richard Cochran wrote:
> On Thu, Aug 23, 2012 at 04:22:02PM +0000, Vick, Matthew wrote:
>
> > > > #ifdef CONFIG_IGB_PTP
> > > > -static int igb_ethtool_get_ts_info(struct net_device *dev,
> > > > +static int igb_get_ts_info(struct net_device *dev,
> > >
> > > I like the old name better.
> >
> > The old name is out of the coding style of igb. Every other
> > function is igb_get_* or igb_set_*, with the exception of
> > igb_ethtool_begin and igb_ethtool_complete.
> No, just leave the names alone, and keep the functions where they
> are. It is just churn.
>
> One of the most useful ways to understand code (at least for me) is to
> use git blame. It tells you when code was added, what the reason was,
> and how the change looks in context. By moving and renaming willy
> nilly, you are obscuring this valuable information.
[]
> > I'm sorry you feel like this patch doesn't improve the driver.
> > The goal is code cleanup and consistency, both of which I
> > consider to be driver improvements and is why I made the patches.
>
> But the code wasn't dirty in the first place. It doesn't need this
> "cleaning." This series undoes the inline-able functions for no good
> reason. As far as ixgbe goes, this driver came first, so you might as
> well be making *that* driver consistent with this one.
I disagree with Richard.
Improving code clarity and consistency isn't churn.
Old code isn't necessarily the best code, nor should
necessarily old code be a required guide for new code.
The most valuable form of code is the current one,
not any antecedent version.
People that need to wade through old crud to blame
someone still can.
^ permalink raw reply
* RE: [PATCH] mwifiex: use is_broadcast_ether_addr() instead of memcmp()
From: Bing Zhao @ 2012-08-24 3:28 UTC (permalink / raw)
To: Wei Yongjun, linville@tuxdriver.com
Cc: yongjun_wei@trendmicro.com.cn, linux-wireless@vger.kernel.org,
netdev@vger.kernel.org
In-Reply-To: <CAPgLHd9YDVVJ+RMZTR4ubQy_cvMPYgWgFWpjpdUuAeu5B6T1Bw@mail.gmail.com>
Hi Wei,
Thanks for the patch again.
> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
>
> Using is_broadcast_ether_addr() instead of directly use
> memcmp() to determine if the ethernet address is broadcast
> address.
>
> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> ---
> drivers/net/wireless/mwifiex/sta_cmd.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/mwifiex/sta_cmd.c b/drivers/net/wireless/mwifiex/sta_cmd.c
> index df3a33c..45461ca 100644
> --- a/drivers/net/wireless/mwifiex/sta_cmd.c
> +++ b/drivers/net/wireless/mwifiex/sta_cmd.c
> @@ -17,6 +17,8 @@
> * this warranty disclaimer.
> */
>
> +#include <linux/etherdevice.h>
This is not needed. One of local .h file has included it already.
This reminds me that your previous patch (is_zero_ether_addr) also had this inclusion.
Could you please resend v2 for both patches?
Thanks,
Bing
> +
> #include "decl.h"
> #include "ioctl.h"
> #include "util.h"
> @@ -551,7 +553,6 @@ mwifiex_cmd_802_11_key_material(struct mwifiex_private *priv,
> struct host_cmd_tlv_mac_addr *tlv_mac;
> u16 key_param_len = 0, cmd_size;
> int ret = 0;
> - const u8 bc_mac[] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
>
> cmd->command = cpu_to_le16(HostCmd_CMD_802_11_KEY_MATERIAL);
> key_material->action = cpu_to_le16(cmd_action);
> @@ -593,7 +594,7 @@ mwifiex_cmd_802_11_key_material(struct mwifiex_private *priv,
> /* set 0 when re-key */
> key_material->key_param_set.key[1] = 0;
>
> - if (0 != memcmp(enc_key->mac_addr, bc_mac, sizeof(bc_mac))) {
> + if (!is_broadcast_ether_addr(enc_key->mac_addr)) {
> /* WAPI pairwise key: unicast */
> key_material->key_param_set.key_info |=
> cpu_to_le16(KEY_UNICAST);
^ permalink raw reply
* RE: [PATCH] mwifiex: use is_zero_ether_addr() instead of memcmp()
From: Bing Zhao @ 2012-08-24 3:30 UTC (permalink / raw)
To: Wei Yongjun
Cc: linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org,
yongjun_wei-zrsr2BFq86L20UzCJQGyNP8+0UxHXcjY@public.gmane.org,
linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CAPgLHd--KY+zNjUwOHKgEJuTfJuoA=7vKR3JMBUgUc+BtjC9Ng-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
> > Apparently there are two more occurrences of zero_mac (scan.c:843)
>
> This is part is special:
>
> memcmp(scan_cfg_out->specific_bssid, &zero_mac, sizeof(zero_mac)))
>
> &zero_mac equal zero_mac, but it missing to spatch.
I will take care of this then.
Thanks,
Bing
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2] net: add new QCA alx ethernet driver
From: Luis R. Rodriguez @ 2012-08-24 4:32 UTC (permalink / raw)
To: Huang, Xiong
Cc: Ben Hutchings, David Miller, Ren, Cloud, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, qca-linux-team, nic-devel
In-Reply-To: <157393863283F442885425D2C45428562A4F5546@nasanexd02f.na.qualcomm.com>
On Fri, Aug 24, 2012 at 6:39 AM, Huang, Xiong <xiong@qca.qualcomm.com> wrote:
It would be useful for you to keep the tag of who wrote what if you
are trimming the e-mails, like in the above you removed the option for
me to easily discover who wrote what.
>> If you want to implement a more sophisticated packet matching mode that
>> requires more details than struct ethtool_wolinfo, then you may need to
>> define new operations.
>>
> It's great !
> Is it ok for ethtool to define a new operation with a sophisticated structure (like ethtool_eee) ?
It would be useful for you to describe the exact functionality,
perhaps in a separate thread or maybe even just RFC patches which you
have to be simply ready for may just be just that -- RFC patches which
we flush down the v2 toilet.
Luis
^ permalink raw reply
* Re: [PATCH net-next v2] net: dev: fix the incorrect hold of net namespace's lo device
From: Eric Dumazet @ 2012-08-24 4:56 UTC (permalink / raw)
To: Gao feng; +Cc: davem, netdev, Eric W. Biederman
In-Reply-To: <1345772215-30379-1-git-send-email-gaofeng@cn.fujitsu.com>
On Fri, 2012-08-24 at 09:36 +0800, Gao feng wrote:
> When moving a net device from one net namespace to another
> net namespace,dev_change_net_namespace calls NETDEV_DOWN
> event,so the original net namespace's dst entries which
> beloned to this net device will be put into dst_garbage
> list.
>
> then dev_change_net_namespace will set this net device's
> net to the new net namespace.
>
> If we unregister this net device's driver, this will trigger
> the NETDEV_UNREGISTER_FINAL event, dst_ifdown will be called,
> and get this net device's dst entries from dst_garbage list,
> put these entries' dev to the new net namespace's lo device.
>
> It's not what we want,actually we need these dst entries hold
> the original net namespace's lo device,this incorrect device
> holding will trigger emg message like below.
> unregister_netdevice: waiting for lo to become free. Usage count = 1
>
> so we should call NETDEV_UNREGISTER_FINAL event in
> dev_change_net_namespace too,in order to make sure dst entries
> already in the dst_garbage list, we need rcu_barrier before we
> call NETDEV_UNREGISTER_FINAL event.
>
> With help form Eric Dumazet.
>
> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
> Cc: Eric Dumazet <eric.dumazet@gmail.com>
> Cc: "Eric W. Biederman" <ebiederm@xmission.com>
> ---
> net/core/dev.c | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index bc857fe..6956d4c 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -6256,6 +6256,8 @@ int dev_change_net_namespace(struct net_device *dev, struct net *net, const char
> the device is just moving and can keep their slaves up.
> */
> call_netdevice_notifiers(NETDEV_UNREGISTER, dev);
> + rcu_barrier();
> + call_netdevice_notifiers(NETDEV_UNREGISTER_FINAL, dev);
> rtmsg_ifinfo(RTM_DELLINK, dev, ~0U);
>
> /*
Acked-by: Eric Dumazet <edumazet@google.com>
Thanks !
^ permalink raw reply
* [PATCH v2] mwifiex: use is_broadcast_ether_addr() instead of memcmp()
From: Wei Yongjun @ 2012-08-24 5:25 UTC (permalink / raw)
To: bzhao-eYqpPyKDWXRBDgjK7y7TUQ, linville-2XuSBdqkA4R54TAoqtyWWQ
Cc: yongjun_wei-zrsr2BFq86L20UzCJQGyNP8+0UxHXcjY,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA
From: Wei Yongjun <yongjun_wei-zrsr2BFq86L20UzCJQGyNP8+0UxHXcjY@public.gmane.org>
Using is_broadcast_ether_addr() instead of directly use
memcmp() to determine if the ethernet address is broadcast
address.
Signed-off-by: Wei Yongjun <yongjun_wei-zrsr2BFq86L20UzCJQGyNP8+0UxHXcjY@public.gmane.org>
---
drivers/net/wireless/mwifiex/sta_cmd.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/wireless/mwifiex/sta_cmd.c b/drivers/net/wireless/mwifiex/sta_cmd.c
index df3a33c..323b91b 100644
--- a/drivers/net/wireless/mwifiex/sta_cmd.c
+++ b/drivers/net/wireless/mwifiex/sta_cmd.c
@@ -551,7 +551,6 @@ mwifiex_cmd_802_11_key_material(struct mwifiex_private *priv,
struct host_cmd_tlv_mac_addr *tlv_mac;
u16 key_param_len = 0, cmd_size;
int ret = 0;
- const u8 bc_mac[] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
cmd->command = cpu_to_le16(HostCmd_CMD_802_11_KEY_MATERIAL);
key_material->action = cpu_to_le16(cmd_action);
@@ -593,7 +592,7 @@ mwifiex_cmd_802_11_key_material(struct mwifiex_private *priv,
/* set 0 when re-key */
key_material->key_param_set.key[1] = 0;
- if (0 != memcmp(enc_key->mac_addr, bc_mac, sizeof(bc_mac))) {
+ if (!is_broadcast_ether_addr(enc_key->mac_addr)) {
/* WAPI pairwise key: unicast */
key_material->key_param_set.key_info |=
cpu_to_le16(KEY_UNICAST);
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v2] mwifiex: use is_zero_ether_addr() instead of memcmp()
From: Wei Yongjun @ 2012-08-24 5:27 UTC (permalink / raw)
To: bzhao, linville; +Cc: yongjun_wei, linux-wireless, netdev
From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
Using is_zero_ether_addr() instead of directly use
memcmp() to determine if the ethernet address is all
zeros.
spatch with a semantic match is used to found this problem.
(http://coccinelle.lip6.fr/)
Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
---
drivers/net/wireless/mwifiex/sta_cmdresp.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/wireless/mwifiex/sta_cmdresp.c b/drivers/net/wireless/mwifiex/sta_cmdresp.c
index 0b09004..62223ab 100644
--- a/drivers/net/wireless/mwifiex/sta_cmdresp.c
+++ b/drivers/net/wireless/mwifiex/sta_cmdresp.c
@@ -736,7 +736,6 @@ static int mwifiex_ret_ibss_coalescing_status(struct mwifiex_private *priv,
{
struct host_cmd_ds_802_11_ibss_status *ibss_coal_resp =
&(resp->params.ibss_coalescing);
- u8 zero_mac[ETH_ALEN] = { 0, 0, 0, 0, 0, 0 };
if (le16_to_cpu(ibss_coal_resp->action) == HostCmd_ACT_GEN_SET)
return 0;
@@ -745,7 +744,7 @@ static int mwifiex_ret_ibss_coalescing_status(struct mwifiex_private *priv,
"info: new BSSID %pM\n", ibss_coal_resp->bssid);
/* If rsp has NULL BSSID, Just return..... No Action */
- if (!memcmp(ibss_coal_resp->bssid, zero_mac, ETH_ALEN)) {
+ if (is_zero_ether_addr(ibss_coal_resp->bssid)) {
dev_warn(priv->adapter->dev, "new BSSID is NULL\n");
return 0;
}
^ permalink raw reply related
* Re: [PATCH v2] net: add new QCA alx ethernet driver
From: Luis R. Rodriguez @ 2012-08-24 5:34 UTC (permalink / raw)
To: Huang, Xiong
Cc: Ben Hutchings, David Miller, Ren, Cloud, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, qca-linux-team, nic-devel
In-Reply-To: <CAB=NE6Xg=NUsb-hQyxr+-RyzpkJ-BH6RbbcTX6KF23H+u4yEPQ@mail.gmail.com>
On Fri, Aug 24, 2012 at 8:32 AM, Luis R. Rodriguez
<rodrigue@qca.qualcomm.com> wrote:
> It would be useful for you to describe the exact functionality,
> perhaps in a separate thread or maybe even just RFC patches which you
> have to be simply ready for may just be just that -- RFC patches which
> we flush down the v2 toilet.
A simple strategy for you is for you to remove this code (if present)
for now and then throw into a crap/ patch [0] under compat-drivers but
with an objective of eventually going through the RFC stages to get
this properly merged upstream.
[0] https://backports.wiki.kernel.org/index.php/Documentation/compat-drivers/additional-patches
Luis
^ permalink raw reply
* RE: [PATCH v2] mwifiex: use is_zero_ether_addr() instead of memcmp()
From: Bing Zhao @ 2012-08-24 6:00 UTC (permalink / raw)
To: Wei Yongjun, linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org
Cc: yongjun_wei-zrsr2BFq86L20UzCJQGyNP8+0UxHXcjY@public.gmane.org,
linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CAPgLHd8+H415tRqvyM1SXQCSC6savCakuFLdjM1ebWvjikd5NQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
> From: Wei Yongjun <yongjun_wei-zrsr2BFq86L20UzCJQGyNP8+0UxHXcjY@public.gmane.org>
>
> Using is_zero_ether_addr() instead of directly use
> memcmp() to determine if the ethernet address is all
> zeros.
>
> spatch with a semantic match is used to found this problem.
> (http://coccinelle.lip6.fr/)
>
> Signed-off-by: Wei Yongjun <yongjun_wei-zrsr2BFq86L20UzCJQGyNP8+0UxHXcjY@public.gmane.org>
Acked-by: Bing Zhao <bzhao-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
Thanks,
Bing
> ---
> drivers/net/wireless/mwifiex/sta_cmdresp.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/mwifiex/sta_cmdresp.c b/drivers/net/wireless/mwifiex/sta_cmdresp.c
> index 0b09004..62223ab 100644
> --- a/drivers/net/wireless/mwifiex/sta_cmdresp.c
> +++ b/drivers/net/wireless/mwifiex/sta_cmdresp.c
> @@ -736,7 +736,6 @@ static int mwifiex_ret_ibss_coalescing_status(struct mwifiex_private *priv,
> {
> struct host_cmd_ds_802_11_ibss_status *ibss_coal_resp =
> &(resp->params.ibss_coalescing);
> - u8 zero_mac[ETH_ALEN] = { 0, 0, 0, 0, 0, 0 };
>
> if (le16_to_cpu(ibss_coal_resp->action) == HostCmd_ACT_GEN_SET)
> return 0;
> @@ -745,7 +744,7 @@ static int mwifiex_ret_ibss_coalescing_status(struct mwifiex_private *priv,
> "info: new BSSID %pM\n", ibss_coal_resp->bssid);
>
> /* If rsp has NULL BSSID, Just return..... No Action */
> - if (!memcmp(ibss_coal_resp->bssid, zero_mac, ETH_ALEN)) {
> + if (is_zero_ether_addr(ibss_coal_resp->bssid)) {
> dev_warn(priv->adapter->dev, "new BSSID is NULL\n");
> return 0;
> }
>
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* RE: [PATCH v2] mwifiex: use is_broadcast_ether_addr() instead of memcmp()
From: Bing Zhao @ 2012-08-24 6:01 UTC (permalink / raw)
To: Wei Yongjun, linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org
Cc: yongjun_wei-zrsr2BFq86L20UzCJQGyNP8+0UxHXcjY@public.gmane.org,
linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CAPgLHd81cZS4K-wzcLycRTVF3pGy-S6=YR9UZ++Ot=6OXZp-5Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
> From: Wei Yongjun <yongjun_wei-zrsr2BFq86L20UzCJQGyNP8+0UxHXcjY@public.gmane.org>
>
> Using is_broadcast_ether_addr() instead of directly use
> memcmp() to determine if the ethernet address is broadcast
> address.
>
> Signed-off-by: Wei Yongjun <yongjun_wei-zrsr2BFq86L20UzCJQGyNP8+0UxHXcjY@public.gmane.org>
Acked-by: Bing Zhao <bzhao-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
Thanks,
Bing
> ---
> drivers/net/wireless/mwifiex/sta_cmd.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/mwifiex/sta_cmd.c b/drivers/net/wireless/mwifiex/sta_cmd.c
> index df3a33c..323b91b 100644
> --- a/drivers/net/wireless/mwifiex/sta_cmd.c
> +++ b/drivers/net/wireless/mwifiex/sta_cmd.c
> @@ -551,7 +551,6 @@ mwifiex_cmd_802_11_key_material(struct mwifiex_private *priv,
> struct host_cmd_tlv_mac_addr *tlv_mac;
> u16 key_param_len = 0, cmd_size;
> int ret = 0;
> - const u8 bc_mac[] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
>
> cmd->command = cpu_to_le16(HostCmd_CMD_802_11_KEY_MATERIAL);
> key_material->action = cpu_to_le16(cmd_action);
> @@ -593,7 +592,7 @@ mwifiex_cmd_802_11_key_material(struct mwifiex_private *priv,
> /* set 0 when re-key */
> key_material->key_param_set.key[1] = 0;
>
> - if (0 != memcmp(enc_key->mac_addr, bc_mac, sizeof(bc_mac))) {
> + if (!is_broadcast_ether_addr(enc_key->mac_addr)) {
> /* WAPI pairwise key: unicast */
> key_material->key_param_set.key_info |=
> cpu_to_le16(KEY_UNICAST);
>
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [net-next 13/13] igb: Store the MAC address in the name in the PTP struct.
From: Richard Cochran @ 2012-08-24 6:19 UTC (permalink / raw)
To: Ben Hutchings
Cc: Jeff Kirsher, davem, Matthew Vick, netdev, gospo, sassmann,
Stuart Hodgson
In-Reply-To: <1345757737.2579.22.camel@bwh-desktop.uk.solarflarecom.com>
On Thu, Aug 23, 2012 at 10:35:37PM +0100, Ben Hutchings wrote:
>
> The name field is described as 'A short name to identify the clock'. It
> is not stated whether this is meant to be the name of the clock *device*
> or the clock *driver*. If it's the name of the device then some unique
> ID such as the permanent MAC address is required.
In an ideal world, there is only one clock device per system, so the
original concept was to use the driver name. The hardware designers
have ignored or not understood the "one clock per system" model, and
so, unfortunately, we have to deal with it.
A clock device can and should be connected to more than one network
device (see gianfar, dp83640), and to give it a MAC address is
misleading.
The ethtool solution works perfectly to go from interface->clock,
which is all the information that a user space PTP stack needs.
I think for sysfs and the ioctl it is nicer to have the driver name,
since it is associated with the clock and its capabilities.
Thanks,
Richard
^ permalink raw reply
* Re: [net-next 11/13] igb: Update PTP function names/variables and locations.
From: Richard Cochran @ 2012-08-24 6:32 UTC (permalink / raw)
To: Joe Perches
Cc: Vick, Matthew, Kirsher, Jeffrey T, davem@davemloft.net,
netdev@vger.kernel.org, gospo@redhat.com, sassmann@redhat.com
In-Reply-To: <1345777345.16471.18.camel@joe2Laptop>
On Thu, Aug 23, 2012 at 08:02:25PM -0700, Joe Perches wrote:
> Improving code clarity and consistency isn't churn.
This patch series moves code around for no good reason. That is, by
definition, churn.
> The most valuable form of code is the current one,
> not any antecedent version.
You really haven't noticed all the code review and rebasing that goes
on around here in order to improve the commit history, have you?
Thanks,
Richard
^ permalink raw reply
* netlink scm creds uid and gids are always 0.
From: Eric W. Biederman @ 2012-08-24 6:45 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
While working on the kuid_t and kgid_t conversion of the audit subsystem
I noticed that since the performance problem of scm creds and af_unix
sockets were fixed af_netlink sockets have not filled in the uid or gid
of the originator of the socket.
I think all we need is an appropriate cred_to_ucred call to fix this
regression, but I am going so many different directions right now I
can't get myself to focus on this long enough to work up an appripriate
patch to fix.
Eric do you think you might take a gander?
The commit where this regression was introduced appears to be.
commit 16e5726269611b71c930054ffe9b858c1cea88eb
Author: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon Sep 19 05:52:27 2011 +0000
af_unix: dont send SCM_CREDENTIALS by default
Since commit 7361c36c5224 (af_unix: Allow credentials to work across
user and pid namespaces) af_unix performance dropped a lot.
This is because we now take a reference on pid and cred in each write(),
and release them in read(), usually done from another process,
eventually from another cpu. This triggers false sharing.
^ permalink raw reply
* Re: [net-next 11/13] igb: Update PTP function names/variables and locations.
From: Richard Cochran @ 2012-08-24 6:55 UTC (permalink / raw)
To: Vick, Matthew
Cc: Keller, Jacob E, Kirsher, Jeffrey T, davem@davemloft.net,
netdev@vger.kernel.org, gospo@redhat.com, sassmann@redhat.com
In-Reply-To: <06DFBC1E25D8024DB214DC7F41A3CD34488DD379@ORSMSX101.amr.corp.intel.com>
On Thu, Aug 23, 2012 at 06:44:24PM +0000, Vick, Matthew wrote:
>
> Isn't this discussion creeping outside the scope of this patch?
Yes, it is. I am suggesting that the driver could use improvement, but
not by renaming functions to suit your or someone else's taste.
Instead, you could work on the time stamping feature.
(And if, along the way, you end up renaming the functions that you
change, then I won't complain ;)
Thanks,
Richard
^ permalink raw reply
* Re: [net-next 10/13] igb: Tidy up wrapping for CONFIG_IGB_PTP.
From: Richard Cochran @ 2012-08-24 7:04 UTC (permalink / raw)
To: Vick, Matthew
Cc: Keller, Jacob E, Kirsher, Jeffrey T, davem@davemloft.net,
netdev@vger.kernel.org, gospo@redhat.com, sassmann@redhat.com
In-Reply-To: <06DFBC1E25D8024DB214DC7F41A3CD34488DD35B@ORSMSX101.amr.corp.intel.com>
On Thu, Aug 23, 2012 at 06:40:25PM +0000, Vick, Matthew wrote:
>
> I tend to agree with Jake here--I like having the information. I'm fine removing them, but I'd like to do it for all CONFIG_IGB_PTP wrapping if we're going to do it. What do you think, Richard?
I took a look at the #ifdefs in igb_main.c, and I would say they are
okay as is, except for the CONFIG_PM ones, which could use a comment
at the #endifs.
(a matter of taste, though)
Thanks,
Richard
^ permalink raw reply
* [PATCH][net-next] gianfar: fix default tx vlan offload feature flag
From: Claudiu Manoil @ 2012-08-24 7:46 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller, Sebastian Poehn, Claudiu Manoil
Commit -
"b852b72 gianfar: fix bug caused by
87c288c6e9aa31720b72e2bc2d665e24e1653c3e"
disables by default (on mac init) the hw vlan tag insertion.
The "features" flags were not updated to reflect this, and
"ethtool -K" shows tx-vlan-offload to be "on" by default.
Cc: Sebastian Poehn <sebastian.poehn@belden.com>
Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com>
---
drivers/net/ethernet/freescale/gianfar.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index 4605f72..d3233f59 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -1041,7 +1041,7 @@ static int gfar_probe(struct platform_device *ofdev)
if (priv->device_flags & FSL_GIANFAR_DEV_HAS_VLAN) {
dev->hw_features |= NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX;
- dev->features |= NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX;
+ dev->features |= NETIF_F_HW_VLAN_RX;
}
if (priv->device_flags & FSL_GIANFAR_DEV_HAS_EXTENDED_HASH) {
--
1.7.6.5
^ permalink raw reply related
* Re: netlink scm creds uid and gids are always 0.
From: Eric Dumazet @ 2012-08-24 7:57 UTC (permalink / raw)
To: Eric W. Biederman; +Cc: netdev
In-Reply-To: <871uiwlrf3.fsf@xmission.com>
On Thu, 2012-08-23 at 23:45 -0700, Eric W. Biederman wrote:
> While working on the kuid_t and kgid_t conversion of the audit subsystem
> I noticed that since the performance problem of scm creds and af_unix
> sockets were fixed af_netlink sockets have not filled in the uid or gid
> of the originator of the socket.
>
> I think all we need is an appropriate cred_to_ucred call to fix this
> regression, but I am going so many different directions right now I
> can't get myself to focus on this long enough to work up an appripriate
> patch to fix.
>
> Eric do you think you might take a gander?
Wasnt it fixed by e0e3cea46d31d23dc40df0a49a7a2c04fe8edfea
af_netlink: force credentials passing [CVE-2012-3520]
Or is it a different thing ?
^ permalink raw reply
* Business
From: hashi @ 2012-08-24 7:31 UTC (permalink / raw)
Good Day,
I am Mr. Xiao Gang from Bank of China.I have a secured business proposal worth
$45,275,000.00. Get back to me for more details.
Regards,
Mr. XIAO Gang
^ permalink raw reply
* Re: netlink scm creds uid and gids are always 0.
From: Eric W. Biederman @ 2012-08-24 8:19 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1345795065.5904.2287.camel@edumazet-glaptop>
Eric Dumazet <eric.dumazet@gmail.com> writes:
> On Thu, 2012-08-23 at 23:45 -0700, Eric W. Biederman wrote:
>> While working on the kuid_t and kgid_t conversion of the audit subsystem
>> I noticed that since the performance problem of scm creds and af_unix
>> sockets were fixed af_netlink sockets have not filled in the uid or gid
>> of the originator of the socket.
>>
>> I think all we need is an appropriate cred_to_ucred call to fix this
>> regression, but I am going so many different directions right now I
>> can't get myself to focus on this long enough to work up an appripriate
>> patch to fix.
>>
>> Eric do you think you might take a gander?
>
> Wasnt it fixed by e0e3cea46d31d23dc40df0a49a7a2c04fe8edfea
>
> af_netlink: force credentials passing [CVE-2012-3520]
>
> Or is it a different thing ?
Same thing. I didn't see that fix go by.
One more little thing I can cross off my list. Hooray!
Eric
^ permalink raw reply
* Re: netlink scm creds uid and gids are always 0.
From: Eric W. Biederman @ 2012-08-24 9:07 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
In-Reply-To: <87txvsk8h7.fsf@xmission.com>
ebiederm@xmission.com (Eric W. Biederman) writes:
> Eric Dumazet <eric.dumazet@gmail.com> writes:
>
>> On Thu, 2012-08-23 at 23:45 -0700, Eric W. Biederman wrote:
>>> While working on the kuid_t and kgid_t conversion of the audit subsystem
>>> I noticed that since the performance problem of scm creds and af_unix
>>> sockets were fixed af_netlink sockets have not filled in the uid or gid
>>> of the originator of the socket.
>>>
>>> I think all we need is an appropriate cred_to_ucred call to fix this
>>> regression, but I am going so many different directions right now I
>>> can't get myself to focus on this long enough to work up an appripriate
>>> patch to fix.
>>>
>>> Eric do you think you might take a gander?
>>
>> Wasnt it fixed by e0e3cea46d31d23dc40df0a49a7a2c04fe8edfea
>>
>> af_netlink: force credentials passing [CVE-2012-3520]
>>
>> Or is it a different thing ?
>
> Same thing. I didn't see that fix go by.
>
> One more little thing I can cross off my list. Hooray!
Looking a little deeper it looks like I am going to have to
give scm credentials a little more tender loving care.
There is still a possible issue with netlink sockets and pids when the
two processes talking over netlink are in different pid namespaces.
And I need to take care in my usernamespace tree for 3.7, to keep from
reintroucing the ability to spoof root if the two netlink talkers are in
different user namespaces.
With a little luck for uids and gids I can just pass around kuid_t and
kgid_t values and throw out the ref-counting complexity. Something
to sleep on and benchmark, and then generate a patch I guess.
I know at least from my last attempt that ref counting in the NETLINK_CB
was a lost cause. So I don't know what to do about the pids :(
Eric
^ permalink raw reply
* Re: [PATCH 1/2] ipv6: do not hold route table lock when send ndisc probe
From: Cong Wang @ 2012-08-24 9:15 UTC (permalink / raw)
To: Eric Dumazet
Cc: Debabrata Banerjee, netdev, Banerjee, Debabrata, David S. Miller,
Hideaki YOSHIFUJI, Patrick McHardy
In-Reply-To: <1345738232.5904.1244.camel@edumazet-glaptop>
On Thu, 2012-08-23 at 18:10 +0200, Eric Dumazet wrote:
> On Tue, 2012-08-21 at 11:44 +0800, Cong Wang wrote:
> > Hi, Debabrata,
> >
> > Could you help to test the attached patch below?
> >
> > Thanks!
> >
>
> Hard to comment on your patch since its not inlined.
>
> + nw = kmalloc(sizeof(*nw), GFP_ATOMIC);
> + if (nw) {
> + memcpy(&nw->target, &neigh->primary_key, sizeof(struct in6_addr));
> + addrconf_addr_solict_mult(&nw->target, &nw->mcaddr);
> + nw->dev = rt->dst.dev;
> + INIT_WORK(&nw->work, queue_ndisc);
> + schedule_work(&nw->work);
> + }
>
> You cant do that without taking extra reference on dev,
> and release it in queue_ndisc()
>
> This also will add interesting side effects at device dismantle.
>
Right... If we call dev_hold() in the work, it is possible that the work
is still not scheduled to running when we unregister the device. What's
more, we can't flush work here as we are holding a read lock.
Hmm.
^ permalink raw reply
* Re: [net-next 11/13] igb: Update PTP function names/variables and locations.
From: Joe Perches @ 2012-08-24 9:22 UTC (permalink / raw)
To: Richard Cochran
Cc: Vick, Matthew, Kirsher, Jeffrey T, davem@davemloft.net,
netdev@vger.kernel.org, gospo@redhat.com, sassmann@redhat.com
In-Reply-To: <20120824063240.GB2212@netboy.at.omicron.at>
On Fri, 2012-08-24 at 08:32 +0200, Richard Cochran wrote:
> On Thu, Aug 23, 2012 at 08:02:25PM -0700, Joe Perches wrote:
>
> > Improving code clarity and consistency isn't churn.
>
> This patch series moves code around for no good reason. That is, by
> definition, churn.
For your definition of good.
> > The most valuable form of code is the current one,
> > not any antecedent version.
>
> You really haven't noticed all the code review and rebasing that goes
> on around here in order to improve the commit history, have you?
That'd be incorrect too.
^ permalink raw reply
* [PATCH 1/2] can: softing: Fix potential memory leak in softing_load_fw()
From: Marc Kleine-Budde @ 2012-08-24 9:30 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-can, Alexey Khoroshilov, Marc Kleine-Budde
In-Reply-To: <1345800643-29456-1-git-send-email-mkl@pengutronix.de>
From: Alexey Khoroshilov <khoroshilov@ispras.ru>
Do not leak memory by updating pointer with potentially NULL realloc return value.
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
drivers/net/can/softing/softing_fw.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/net/can/softing/softing_fw.c b/drivers/net/can/softing/softing_fw.c
index 3105961..b595d34 100644
--- a/drivers/net/can/softing/softing_fw.c
+++ b/drivers/net/can/softing/softing_fw.c
@@ -150,7 +150,7 @@ int softing_load_fw(const char *file, struct softing *card,
const uint8_t *mem, *end, *dat;
uint16_t type, len;
uint32_t addr;
- uint8_t *buf = NULL;
+ uint8_t *buf = NULL, *new_buf;
int buflen = 0;
int8_t type_end = 0;
@@ -199,11 +199,12 @@ int softing_load_fw(const char *file, struct softing *card,
if (len > buflen) {
/* align buflen */
buflen = (len + (1024-1)) & ~(1024-1);
- buf = krealloc(buf, buflen, GFP_KERNEL);
- if (!buf) {
+ new_buf = krealloc(buf, buflen, GFP_KERNEL);
+ if (!new_buf) {
ret = -ENOMEM;
goto failed;
}
+ buf = new_buf;
}
/* verify record data */
memcpy_fromio(buf, &dpram[addr + offset], len);
--
1.7.10
^ permalink raw reply related
* [PATCH 2/2] can: sja1000_platform: fix wrong flag IRQF_SHARED for interrupt sharing
From: Marc Kleine-Budde @ 2012-08-24 9:30 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-can, Sven Schmitt, Marc Kleine-Budde
In-Reply-To: <1345800643-29456-1-git-send-email-mkl@pengutronix.de>
From: Sven Schmitt <sven.schmitt@volkswagen.de>
The sja1000 platform driver wrongly assumes that a shared IRQ is indicated
with the IRQF_SHARED flag in irq resource flags. This patch changes the
driver to handle the correct flag IORESOURCE_IRQ_SHAREABLE instead.
There are no mainline users of the platform driver which wrongly make use
of IRQF_SHARED.
Signed-off-by: Sven Schmitt <sven.schmitt@volkswagen.de>
Acked-by: Yegor Yefremov <yegorslists@googlemail.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
drivers/net/can/sja1000/sja1000_platform.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/can/sja1000/sja1000_platform.c b/drivers/net/can/sja1000/sja1000_platform.c
index 4f50145..662c5f7 100644
--- a/drivers/net/can/sja1000/sja1000_platform.c
+++ b/drivers/net/can/sja1000/sja1000_platform.c
@@ -109,7 +109,9 @@ static int sp_probe(struct platform_device *pdev)
priv = netdev_priv(dev);
dev->irq = res_irq->start;
- priv->irq_flags = res_irq->flags & (IRQF_TRIGGER_MASK | IRQF_SHARED);
+ priv->irq_flags = res_irq->flags & IRQF_TRIGGER_MASK;
+ if (res_irq->flags & IORESOURCE_IRQ_SHAREABLE)
+ priv->irq_flags |= IRQF_SHARED;
priv->reg_base = addr;
/* The CAN clock frequency is half the oscillator clock frequency */
priv->can.clock.freq = pdata->osc_freq / 2;
--
1.7.10
^ permalink raw reply related
* pull-request: can 2012-08-24
From: Marc Kleine-Budde @ 2012-08-24 9:30 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-can
Hello David,
here are two fixes for the v3.6 release cycle. Alexey Khoroshilov submitted a
fix for a memory leak in the softing driver (in softing_load_fw()) in case a
krealloc() fails. Sven Schmitt fixed the misuse of the IRQF_SHARED flag in the
irq resouce of the sja1000 platform driver, now the correct flag is used. There
are no mainline users of this feature which need to be converted.
regards, Marc
The following changes since commit a0dfb2634e5671770f598cda08002d8cda66ac77:
af_packet: match_fanout_group() can be static (2012-08-23 09:27:12 -0700)
are available in the git repository at:
git://gitorious.org/linux-can/linux-can.git fixes-for-3.6
for you to fetch changes up to da3d50ef308d53f216f1f92f4971f245c13e9f65:
can: sja1000_platform: fix wrong flag IRQF_SHARED for interrupt sharing (2012-08-24 10:54:05 +0200)
----------------------------------------------------------------
Alexey Khoroshilov (1):
can: softing: Fix potential memory leak in softing_load_fw()
Sven Schmitt (1):
can: sja1000_platform: fix wrong flag IRQF_SHARED for interrupt sharing
drivers/net/can/sja1000/sja1000_platform.c | 4 +++-
drivers/net/can/softing/softing_fw.c | 7 ++++---
2 files changed, 7 insertions(+), 4 deletions(-)
^ 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