* Re: [PATCH net-next] net/mlx4_en: remove redundant code
From: Amir Vadai @ 2013-01-20 14:48 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S. Miller, netdev, Or Gerlitz, Yevgeny Petrilin,
Eugenia Emantayev
In-Reply-To: <1358436367.29723.49.camel@edumazet-glaptop>
On 17/01/2013 17:26, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> remove redundant code from build_inline_wqe()
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> ---
> Amir, reviewing this driver, it looks like following could be done,
> could you test the patch for me ?
>
> Thanks
>
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_tx.c b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
> index 2b799f4..16af338 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_tx.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
> @@ -515,10 +515,6 @@ static void build_inline_wqe(struct mlx4_en_tx_desc *tx_desc, struct sk_buff *sk
> wmb();
> inl->byte_count = cpu_to_be32(1 << 31 | (skb->len - spc));
> }
> - tx_desc->ctrl.vlan_tag = cpu_to_be16(*vlan_tag);
> - tx_desc->ctrl.ins_vlan = MLX4_WQE_CTRL_INS_VLAN *
> - (!!vlan_tx_tag_present(skb));
> - tx_desc->ctrl.fence_size = (real_size / 16) & 0x3f;
> }
>
> u16 mlx4_en_select_queue(struct net_device *dev, struct sk_buff *skb)
>
>
Acked-By: Amir Vadai <amirv@mellanox.com>
^ permalink raw reply
* Re: IPsec AH use of ahash
From: Tom St Denis @ 2013-01-20 15:07 UTC (permalink / raw)
To: Mike Galbraith
Cc: Eric Dumazet, Waskiewicz Jr, Peter P, David Miller,
steffen klassert, herbert, linux-kernel, netdev, Michal Kubecek
In-Reply-To: <1358691094.5705.40.camel@marge.simpson.net>
----- Original Message -----
> From: "Mike Galbraith" <bitbucket@online.de>
> To: "Tom St Denis" <tstdenis@elliptictech.com>
> Cc: "Eric Dumazet" <erdnetdev@gmail.com>, "Waskiewicz Jr, Peter P" <peter.p.waskiewicz.jr@intel.com>, "David Miller"
> <davem@davemloft.net>, "steffen klassert" <steffen.klassert@secunet.com>, herbert@gondor.apana.org.au,
> linux-kernel@vger.kernel.org, netdev@vger.kernel.org, "Michal Kubecek" <mkubecek@suse.cz>
> Sent: Sunday, 20 January, 2013 9:11:34 AM
> Subject: Re: IPsec AH use of ahash
>
> On Sun, 2013-01-20 at 07:55 -0500, Tom St Denis wrote:
> >
> > ----- Original Message -----
> > > From: "Mike Galbraith" <bitbucket@online.de>
> > > To: "Tom St Denis" <tstdenis@elliptictech.com>
> > > Cc: "Eric Dumazet" <erdnetdev@gmail.com>, "Waskiewicz Jr, Peter
> > > P" <peter.p.waskiewicz.jr@intel.com>, "David Miller"
> > > <davem@davemloft.net>, "steffen klassert"
> > > <steffen.klassert@secunet.com>, herbert@gondor.apana.org.au,
> > > linux-kernel@vger.kernel.org, netdev@vger.kernel.org, "Michal
> > > Kubecek" <mkubecek@suse.cz>
> > > Sent: Sunday, 20 January, 2013 12:06:21 AM
> > > Subject: Re: IPsec AH use of ahash
> > >
> > > On Sat, 2013-01-19 at 05:30 -0500, Tom St Denis wrote:
> > >
> > > > For those of us who do Kernel development during business hours
> > > > it's
> > > > hard to justify the work when the path to mainline is
> > > > convoluted
> > > > and
> > > > landmined.
> > >
> > > Sounds as though any patches you submit land on your dinner plate
> > > just
> > > like potatoes. Hand the cook a pot of half peeled potatoes,
> > > he/she
> > > may
> > > say try again. The result of a little extra effort is tastier
> > > taters
> > > for everybody feasting at the common table.. including you.
> >
> > No, in reality what happened is the chef made potatos [incorrectly]
> > got busy and asked others to help out and make more potatos. Then
> > came back and said ...
>
> Bottom line: either you grit your teeth and try again or you don't.
> Calling the chef a big meanie doesn't put taters on dinner plates.
One point you're missing is that *I* have CMAC support. *YOU* don't. So being contrary and adversarial about it isn't really hurting me, it's annoying me because I have to manually supply a patch for my users but at the end of the day it isn't me who is losing out. It seems odd that the maintainers who should be happy to receive original content which adds standards support are so ardent that it must be done perfectly and totally unlike the code they submit (which I've already shown doesn't meet these standards).
The other point is that the system can use some working on and unless people raise concerns nothing will change. Large swathes of kernel code don't meet these "coding standards" despite the fact that many source files that are in violation have been "worked on" long after the checkpatch script was written.
In all likelihood I will submit a revised CMAC patch but it'll take time before I can get business hours to work on it. So instead of having a maintainer just touch it up we're all going to lose out because of pride?
Tom
^ permalink raw reply
* Re: [PATCH] firewire net: Ensure checksumming in upper layer.
From: YOSHIFUJI Hideaki @ 2013-01-20 15:10 UTC (permalink / raw)
To: stephan.gatzka
Cc: Stefan Richter, linux1394-devel, netdev, linux-kernel,
YOSHIFUJI Hideaki
In-Reply-To: <50FC0353.8090902@gmail.com>
Stephan Gatzka wrote:
>
>>> Indeed neither the device nor the lower drivers check protocol checksums.
>>> But the CRCs of the encapsulating 1394 packets are checked in hardware.
>>> Shall protocol checksums be verified regardless?
>>
>> Yes, because packets may come from off-link source.
>>
>
> Hm, I can't see any off-link packets coming from fwnet_finish_incoming_packet()
>
> So I wont verify checksums in the driver.
"Off-link source" means the source exists on the different L2
network. In other words, source is connected via router(s).
ethernet firewire
Host -------------- Router ------------ Host
--yoshfuji
^ permalink raw reply
* Re: [PATCH] firewire net: Ensure checksumming in upper layer.
From: Stephan Gatzka @ 2013-01-20 15:17 UTC (permalink / raw)
To: YOSHIFUJI Hideaki; +Cc: netdev, Stefan Richter, linux1394-devel, linux-kernel
In-Reply-To: <50FC08E5.6020903@linux-ipv6.org>
> "Off-link source" means the source exists on the different L2
> network. In other words, source is connected via router(s).
>
> ethernet firewire
> Host -------------- Router ------------ Host
>
O.k., understood. But the receiving router verifies the checksum of
incoming packets and sends them on the firewire link. On firewire we
have CRC checksums to ensure the integrity of packets.
I agree with your patch but I don't see why we should check them in the
driver. I thought your patch will ensure that the checksums will be
verified in the upper layers.
Stephan
------------------------------------------------------------------------------
Master Visual Studio, SharePoint, SQL, ASP.NET, C# 2012, HTML5, CSS,
MVC, Windows 8 Apps, JavaScript and much more. Keep your skills current
with LearnDevNow - 3,200 step-by-step video tutorials by Microsoft
MVPs and experts. ON SALE this month only -- learn more at:
http://p.sf.net/sfu/learnmore_123012
^ permalink raw reply
* Re: [PATCH] firewire net: Ensure checksumming in upper layer.
From: YOSHIFUJI Hideaki @ 2013-01-20 15:48 UTC (permalink / raw)
To: stephan.gatzka
Cc: YOSHIFUJI Hideaki, netdev, Stefan Richter, linux1394-devel,
linux-kernel
In-Reply-To: <50FC0A6C.205@gmail.com>
Stephan Gatzka wrote:
>
>> "Off-link source" means the source exists on the different L2
>> network. In other words, source is connected via router(s).
>>
>> ethernet firewire
>> Host -------------- Router ------------ Host
>>
>
> O.k., understood. But the receiving router verifies the checksum of incoming packets and sends them on the firewire link. On firewire we have CRC checksums to ensure the integrity of packets.
>
> I agree with your patch but I don't see why we should check them in the driver. I thought your patch will ensure that the checksums will be verified in the upper layers.
Routers do not inspect whole packet.
For IPv4, we have IP checksum, but routers (usually) do not check
upper-layer (e.g. UDP) checksum.
For IPv6, we do not have IP checksum.
CHECKSUM_UNNECESSARY means the driver has verified upper layer
(e.g. TCP/UDP) checksum. Modern hardware can perform upper-layer
checksumming as well. But of course, not all drivers are required
to verify the upper-layer checksum; if the driver do not verify
checksum in the packet, just say CHECKSUM_NONE.
Regards,
--yoshfuji
------------------------------------------------------------------------------
Master Visual Studio, SharePoint, SQL, ASP.NET, C# 2012, HTML5, CSS,
MVC, Windows 8 Apps, JavaScript and much more. Keep your skills current
with LearnDevNow - 3,200 step-by-step video tutorials by Microsoft
MVPs and experts. ON SALE this month only -- learn more at:
http://p.sf.net/sfu/learnmore_123012
^ permalink raw reply
* [patch v3] b43: N-PHY: fix gain in b43_nphy_get_gain_ctl_workaround_ent()
From: Dan Carpenter @ 2013-01-20 16:31 UTC (permalink / raw)
To: Stefano Brivio
Cc: John W. Linville, linux-wireless, b43-dev, netdev, linux-kernel,
David Laight, kernel-janitors
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B7118@saturn3.aculab.com>
There were no break statements in this switch statement so everything
used the default settings. Per Walter Harms's suggestion, I've replaced
the switch statement and done a little cleanup.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
v2: Make additional style fixes as well while we're messing with the
function.
v3: Make the array static const int.
diff --git a/drivers/net/wireless/b43/tables_nphy.c b/drivers/net/wireless/b43/tables_nphy.c
index 97d4e27..aaca60c 100644
--- a/drivers/net/wireless/b43/tables_nphy.c
+++ b/drivers/net/wireless/b43/tables_nphy.c
@@ -3226,8 +3226,6 @@ struct nphy_gain_ctl_workaround_entry *b43_nphy_get_gain_ctl_workaround_ent(
{
struct nphy_gain_ctl_workaround_entry *e;
u8 phy_idx;
- u8 tr_iso = ghz5 ? dev->dev->bus_sprom->fem.ghz5.tr_iso :
- dev->dev->bus_sprom->fem.ghz2.tr_iso;
if (!ghz5 && dev->phy.rev >= 6 && dev->phy.radio_rev == 11)
return &nphy_gain_ctl_wa_phy6_radio11_ghz2;
@@ -3249,6 +3247,10 @@ struct nphy_gain_ctl_workaround_entry *b43_nphy_get_gain_ctl_workaround_ent(
!b43_channel_type_is_40mhz(dev->phy.channel_type))
e->cliplo_gain = 0x2d;
} else if (!ghz5 && dev->phy.rev >= 5) {
+ static const int gain_data[] = {0x0062, 0x0064, 0x006a, 0x106a,
+ 0x106c, 0x1074, 0x107c, 0x207c};
+ u8 tr_iso = dev->dev->bus_sprom->fem.ghz2.tr_iso;
+
if (ext_lna) {
e->rfseq_init[0] &= ~0x4000;
e->rfseq_init[1] &= ~0x4000;
@@ -3256,26 +3258,10 @@ struct nphy_gain_ctl_workaround_entry *b43_nphy_get_gain_ctl_workaround_ent(
e->rfseq_init[3] &= ~0x4000;
e->init_gain &= ~0x4000;
}
- switch (tr_iso) {
- case 0:
- e->cliplo_gain = 0x0062;
- case 1:
- e->cliplo_gain = 0x0064;
- case 2:
- e->cliplo_gain = 0x006a;
- case 3:
- e->cliplo_gain = 0x106a;
- case 4:
- e->cliplo_gain = 0x106c;
- case 5:
- e->cliplo_gain = 0x1074;
- case 6:
- e->cliplo_gain = 0x107c;
- case 7:
- e->cliplo_gain = 0x207c;
- default:
- e->cliplo_gain = 0x106a;
- }
+ if (tr_iso > 7)
+ tr_iso = 3;
+ e->cliplo_gain = gain_data[tr_iso];
+
} else if (ghz5 && dev->phy.rev == 4 && ext_lna) {
e->rfseq_init[0] &= ~0x4000;
e->rfseq_init[1] &= ~0x4000;
^ permalink raw reply related
* Re: IPsec AH use of ahash
From: David Dillow @ 2013-01-20 16:34 UTC (permalink / raw)
To: Tom St Denis; +Cc: linux-kernel, netdev
In-Reply-To: <2012247681.93123.1358694444237.JavaMail.root@elliptictech.com>
On Sun, 2013-01-20 at 10:07 -0500, Tom St Denis wrote:
> In all likelihood I will submit a revised CMAC patch but it'll take
> time before I can get business hours to work on it. So instead of
> having a maintainer just touch it up we're all going to lose out
> because of pride?
Yes -- but it would seem to be yours that presents the problem.
I'm sure you could have fixed your patch up in the amount of time you've
spent railing against the push-back. How much of that were you billing
for, and is that a productive use your employer's money? Even on your
own time, how productive has that been? Do you feel better, having
vented?
You don't think the maintainers are maintaining if they don't clean up
after every random contributor -- fine, call them lieutenants then.
Their job is to guide the new development towards the goals for the
system, review patches, and develop new features.
You'll notice I didn't say "reformat code when standards change." This
is a distributed project and it doesn't scale to have the code
continually in flux -- reformatting creates conflicts in other
contributors patches, which then consume all of a lieutenant's time if
they were to try to fix up each one for them. It's much better to push
push that work out to the edge of the network -- put another way, "many
hands make light work." Fixing existing formatting problems is
acceptable in a patch series if one is working in the area, but it is
rare that a series devoted solely to that kind of cleanup gets in.
The coding styles have evolved over time, and are different for
different areas of the kernel -- for example, most of the kernel wants
'/*' on its own line for a multi-line comment, but under net/ it should
not. You should try to match the style in your area -- claiming that the
cryto/ code does it one way is unlikely to sway opinion in net/.
Different lieutenants, different opinions.
As for the existing style issues in net/ipv4/ah4.c you posted, no, your
patch adding a feature in ah4 would probably not been rejected because
of the existing issues, though if one was in the middle of your changes,
it is possible that your would be asked to fix it. As for the issues
checkpatch.pl found, they are in many cases legitimate, but are also the
exceptions -- there are plenty of counter-examples in that file showing
the preferred style.
All the best for your future endeavors,
Dave
^ permalink raw reply
* Re: IPsec AH use of ahash
From: H. Peter Anvin @ 2013-01-20 17:03 UTC (permalink / raw)
To: Tom St Denis
Cc: Mike Galbraith, Eric Dumazet, Waskiewicz Jr, Peter P,
David Miller, steffen klassert, herbert, linux-kernel, netdev,
Michal Kubecek
In-Reply-To: <2012247681.93123.1358694444237.JavaMail.root@elliptictech.com>
On 01/20/2013 07:07 AM, Tom St Denis wrote:
>
> In all likelihood I will submit a revised CMAC patch but it'll take
> time before I can get business hours to work on it. So instead of
> having a maintainer just touch it up we're all going to lose out
> because of pride?
>
It's not about pride. It is about the fact that maintainers don't
scale. A single troublesome contributor can easily take up as much
maintainer time as over a dozen contributors who know how to work well
with their upstream.
-hpa
--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel. I don't speak on their behalf.
^ permalink raw reply
* Re: [PATCH 2/2] CDC_NCM: adding support FLAG_NOARP for Infineon modem platform
From: Sergei Shtylyov @ 2013-01-20 17:13 UTC (permalink / raw)
To: Wei Shuai
Cc: dcbw-H+wXaHxf7aLQT0dZR+AlfA, davem-fT/PcQaiUtIeIZ0/mPfg9Q,
peter-Y+HMSxxDrH8, oneukum-l3A5Bk7waGM,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
alexey.orishko-0IS4wlFg1OjSUeElwK9/Pw, bjorn-yOkvZcmFvRU,
linux-usb-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1358662329-8294-2-git-send-email-cpuwolf-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Hello.
On 20-01-2013 10:12, Wei Shuai wrote:
> Infineon(now Intel) HSPA Modem platform NCM cannot support ARP. we can define a new common structure wwan_noarp_info.
Wrap your lines at 76-80 columns maximum please.
> Then more similiar NO ARP devices can be handled easily
> Signed-off-by: Wei Shuai <cpuwolf-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> drivers/net/usb/cdc_ncm.c | 21 +++++++++++++++++++++
> 1 files changed, 21 insertions(+), 0 deletions(-)
> diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
> index 71b6e92..2d699b6 100644
> --- a/drivers/net/usb/cdc_ncm.c
> +++ b/drivers/net/usb/cdc_ncm.c
> @@ -1155,6 +1155,20 @@ static const struct driver_info wwan_info = {
> .tx_fixup = cdc_ncm_tx_fixup,
> };
>
> +/* Same as wwan_info, but with IFF_NOARP */
FLAG_NOARP, you mean?
> +static const struct driver_info wwan_noarp_info = {
> + .description = "Mobile Broadband Network Device (NO ARP)",
> + .flags = FLAG_POINTTOPOINT | FLAG_NO_SETINT | FLAG_MULTI_PACKET
> + | FLAG_WWAN | FLAG_NOARP,
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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: IPsec AH use of ahash
From: Tom St Denis @ 2013-01-20 17:33 UTC (permalink / raw)
To: H. Peter Anvin
Cc: Mike Galbraith, Eric Dumazet, Waskiewicz Jr, Peter P,
David Miller, steffen klassert, herbert, linux-kernel, netdev,
Michal Kubecek
In-Reply-To: <50FC2366.1000509@zytor.com>
----- Original Message -----
> From: "H. Peter Anvin" <hpa@zytor.com>
> To: "Tom St Denis" <tstdenis@elliptictech.com>
> Cc: "Mike Galbraith" <bitbucket@online.de>, "Eric Dumazet" <erdnetdev@gmail.com>, "Waskiewicz Jr, Peter P"
> <peter.p.waskiewicz.jr@intel.com>, "David Miller" <davem@davemloft.net>, "steffen klassert"
> <steffen.klassert@secunet.com>, herbert@gondor.hengli.com.au, linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
> "Michal Kubecek" <mkubecek@suse.cz>
> Sent: Sunday, 20 January, 2013 12:03:34 PM
> Subject: Re: IPsec AH use of ahash
>
> On 01/20/2013 07:07 AM, Tom St Denis wrote:
> >
> > In all likelihood I will submit a revised CMAC patch but it'll take
> > time before I can get business hours to work on it. So instead of
> > having a maintainer just touch it up we're all going to lose out
> > because of pride?
> >
>
> It's not about pride. It is about the fact that maintainers don't
> scale. A single troublesome contributor can easily take up as much
> maintainer time as over a dozen contributors who know how to work
> well
> with their upstream.
Ironically I'd view consistency with existing code as paramount over [say] adherence to some coding standard that none of the code I've seen in the kernel apparently sticks to in the first place. In this case since XCBC and CMAC operate almost identically it made sense to me to copy it as a template. Now you're telling me I have to re-write it... so that now it's different than XCBC? Or are you suggesting that I also re-write XCBC?
Similarly AH4 and AH6 violate the coding standards. Are you suggesting I re-write those entirely as well to merely augment its functionality?
For a project that *boasts* about it's abhorrent lack of commenting/documentation since "the source is the documentation" it's funny that you can't actually READ the source as an authority.
Tom
^ permalink raw reply
* [PATCH net-next 0/5] ADDRCONF/NDISC improvements
From: YOSHIFUJI Hideaki @ 2013-01-20 17:38 UTC (permalink / raw)
To: davem, netdev; +Cc: yoshfuji
These are for address comparison improvements and boolean conversion.
YOSHIFUJI Hideaki (5):
ipv6: Make ipv6_addr_is_XXX() return boolean.
ipv6: Introduce ipv6_addr_is_solict_mult() to check Solicited Node
Multicast Addresses.
ipv6: Optimize ipv6_addr_is_solict_mult().
ipv6: Optimize ipv6_addr_is_ll_all_{nodes,routers}().
ndisc: Make several arguments for ndisc_send_na() boolean.
include/net/addrconf.h | 33 +++++++++++++++++++++++++++++----
net/ipv6/ndisc.c | 16 ++++++----------
2 files changed, 35 insertions(+), 14 deletions(-)
--
1.7.9.5
^ permalink raw reply
* [PATCH net-next 4/5] ipv6: Optimize ipv6_addr_is_ll_all_{nodes,routers}().
From: YOSHIFUJI Hideaki @ 2013-01-20 17:38 UTC (permalink / raw)
To: davem, netdev; +Cc: yoshfuji
Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
include/net/addrconf.h | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/include/net/addrconf.h b/include/net/addrconf.h
index 9dc5efc..6c58d50 100644
--- a/include/net/addrconf.h
+++ b/include/net/addrconf.h
@@ -288,16 +288,26 @@ static inline bool ipv6_addr_is_multicast(const struct in6_addr *addr)
static inline bool ipv6_addr_is_ll_all_nodes(const struct in6_addr *addr)
{
+#if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) && BITS_PER_LONG == 64
+ __u64 *p = (__u64 *)addr;
+ return ((p[0] ^ cpu_to_be64(0xff02000000000000UL)) | (p[1] ^ cpu_to_be64(1))) == 0UL;
+#else
return ((addr->s6_addr32[0] ^ htonl(0xff020000)) |
addr->s6_addr32[1] | addr->s6_addr32[2] |
(addr->s6_addr32[3] ^ htonl(0x00000001))) == 0;
+#endif
}
static inline bool ipv6_addr_is_ll_all_routers(const struct in6_addr *addr)
{
+#if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) && BITS_PER_LONG == 64
+ __u64 *p = (__u64 *)addr;
+ return ((p[0] ^ cpu_to_be64(0xff02000000000000UL)) | (p[1] ^ cpu_to_be64(2))) == 0UL;
+#else
return ((addr->s6_addr32[0] ^ htonl(0xff020000)) |
addr->s6_addr32[1] | addr->s6_addr32[2] |
(addr->s6_addr32[3] ^ htonl(0x00000002))) == 0;
+#endif
}
static inline bool ipv6_addr_is_isatap(const struct in6_addr *addr)
--
1.7.9.5
^ permalink raw reply related
* [PATCH net-next 2/5] ipv6: Introduce ipv6_addr_is_solict_mult() to check Solicited Node Multicast Addresses.
From: YOSHIFUJI Hideaki @ 2013-01-20 17:39 UTC (permalink / raw)
To: davem, netdev; +Cc: yoshfuji
Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
include/net/addrconf.h | 8 ++++++++
net/ipv6/ndisc.c | 6 +-----
2 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/include/net/addrconf.h b/include/net/addrconf.h
index c6a4421..3a3eeb4 100644
--- a/include/net/addrconf.h
+++ b/include/net/addrconf.h
@@ -305,6 +305,14 @@ static inline bool ipv6_addr_is_isatap(const struct in6_addr *addr)
return (addr->s6_addr32[2] | htonl(0x02000000)) == htonl(0x02005EFE);
}
+static inline bool ipv6_addr_is_solict_mult(const struct in6_addr *addr)
+{
+ return (addr->s6_addr32[0] == htonl(0xff020000) &&
+ addr->s6_addr32[1] == htonl(0x00000000) &&
+ addr->s6_addr32[2] == htonl(0x00000001) &&
+ addr->s6_addr[12] == 0xff);
+}
+
#ifdef CONFIG_PROC_FS
extern int if6_proc_init(void);
extern void if6_proc_exit(void);
diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index 350f860..903191a 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -685,11 +685,7 @@ static void ndisc_recv_ns(struct sk_buff *skb)
* RFC2461 7.1.1:
* DAD has to be destined for solicited node multicast address.
*/
- if (dad &&
- !(daddr->s6_addr32[0] == htonl(0xff020000) &&
- daddr->s6_addr32[1] == htonl(0x00000000) &&
- daddr->s6_addr32[2] == htonl(0x00000001) &&
- daddr->s6_addr [12] == 0xff )) {
+ if (dad && !ipv6_addr_is_solict_mult(daddr)) {
ND_PRINTK(2, warn, "NS: bad DAD packet (wrong destination)\n");
return;
}
--
1.7.9.5
^ permalink raw reply related
* [PATCH net-next 3/5] ipv6: Optimize ipv6_addr_is_solict_mult().
From: YOSHIFUJI Hideaki @ 2013-01-20 17:39 UTC (permalink / raw)
To: davem, netdev; +Cc: yoshfuji
Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
include/net/addrconf.h | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/include/net/addrconf.h b/include/net/addrconf.h
index 3a3eeb4..9dc5efc 100644
--- a/include/net/addrconf.h
+++ b/include/net/addrconf.h
@@ -307,10 +307,17 @@ static inline bool ipv6_addr_is_isatap(const struct in6_addr *addr)
static inline bool ipv6_addr_is_solict_mult(const struct in6_addr *addr)
{
- return (addr->s6_addr32[0] == htonl(0xff020000) &&
- addr->s6_addr32[1] == htonl(0x00000000) &&
- addr->s6_addr32[2] == htonl(0x00000001) &&
- addr->s6_addr[12] == 0xff);
+#if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) && BITS_PER_LONG == 64
+ __u64 *p = (__u64 *)addr;
+ return ((p[0] ^ cpu_to_be64(0xff02000000000000UL)) |
+ ((p[1] ^ cpu_to_be64(0x00000001ff000000UL)) &
+ cpu_to_be64(0xffffffffff000000UL))) == 0UL;
+#else
+ return ((addr->s6_addr32[0] ^ htonl(0xff020000)) |
+ addr->s6_addr32[1] |
+ (addr->s6_addr32[2] ^ htonl(0x00000001)) |
+ (addr->s6_addr[12] ^ 0xff)) == 0;
+#endif
}
#ifdef CONFIG_PROC_FS
--
1.7.9.5
^ permalink raw reply related
* [PATCH net-next 5/5] ndisc: Make several arguments for ndisc_send_na() boolean.
From: YOSHIFUJI Hideaki @ 2013-01-20 17:39 UTC (permalink / raw)
To: davem, netdev; +Cc: yoshfuji
Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
net/ipv6/ndisc.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index 903191a..067a0d2 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -490,7 +490,7 @@ static void __ndisc_send(struct net_device *dev,
static void ndisc_send_na(struct net_device *dev, struct neighbour *neigh,
const struct in6_addr *daddr,
const struct in6_addr *solicited_addr,
- int router, int solicited, int override, int inc_opt)
+ bool router, bool solicited, bool override, bool inc_opt)
{
struct in6_addr tmpaddr;
struct inet6_ifaddr *ifp;
@@ -776,11 +776,11 @@ static void ndisc_recv_ns(struct sk_buff *skb)
}
if (is_router < 0)
- is_router = !!idev->cnf.forwarding;
+ is_router = idev->cnf.forwarding;
if (dad) {
ndisc_send_na(dev, NULL, &in6addr_linklocal_allnodes, &msg->target,
- is_router, 0, (ifp != NULL), 1);
+ !!is_router, false, (ifp != NULL), true);
goto out;
}
@@ -801,8 +801,8 @@ static void ndisc_recv_ns(struct sk_buff *skb)
NEIGH_UPDATE_F_OVERRIDE);
if (neigh || !dev->header_ops) {
ndisc_send_na(dev, neigh, saddr, &msg->target,
- is_router,
- 1, (ifp != NULL && inc), inc);
+ !!is_router,
+ true, (ifp != NULL && inc), inc);
if (neigh)
neigh_release(neigh);
}
--
1.7.9.5
^ permalink raw reply related
* Re: IPsec AH use of ahash
From: Tom St Denis @ 2013-01-20 17:40 UTC (permalink / raw)
To: David Dillow; +Cc: linux-kernel, netdev
In-Reply-To: <1358699692.2494.29.camel@obelisk.thedillows.org>
----- Original Message -----
> From: "David Dillow" <dave@thedillows.org>
> To: "Tom St Denis" <tstdenis@elliptictech.com>
> Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org
> Sent: Sunday, 20 January, 2013 11:34:52 AM
> Subject: Re: IPsec AH use of ahash
>
> On Sun, 2013-01-20 at 10:07 -0500, Tom St Denis wrote:
> > In all likelihood I will submit a revised CMAC patch but it'll take
> > time before I can get business hours to work on it. So instead of
> > having a maintainer just touch it up we're all going to lose out
> > because of pride?
>
> Yes -- but it would seem to be yours that presents the problem.
Not really. Again, *I* have this content. You do not. And the only reason you don't is because someone with pull-request authority is too lazy [or apathetic] to make minor cosmetic changes and get the change pushed through the process.
> I'm sure you could have fixed your patch up in the amount of time
> you've
> spent railing against the push-back. How much of that were you
> billing
> for, and is that a productive use your employer's money? Even on your
> own time, how productive has that been? Do you feel better, having
> vented?
I was hoping to have a constructive conversation in which the maintainers could come clean about the complete hypocrisy of claiming that the source is the reference [since there is no documentation for anything in the kernel] and then saying you can't actually read the source when you want to contribute. Then to tell me I have to adhere to some coding standard (which all things aside I don't agree with in the first place) despite the fact that *none* of the code in the kernel complies with it.
Furthermore I don't think you get how the business side of this works.
I was tasked with getting CMAC to work with IPsec. I did. I sent the patch off to the LKML. My boss tasked me with other work (totally unrelated to IPsec) since in their eyes the project was done [we do after all have CMAC support].
To then go back to my boss and say I need another couple of hours to "clean" it, re-test it in our IPsec lab and then resubmit it in hopes that the "maintainers" don't find some other reason to reject it is totally unprofessional and unproductive.
> You don't think the maintainers are maintaining if they don't clean
> up
> after every random contributor -- fine, call them lieutenants then.
> Their job is to guide the new development towards the goals for the
> system, review patches, and develop new features.
Know this. Being a software developer involves doing a lot of lowly work that most people aren't excited by. You guys already don't comment/document your code. It's unreasonable to assume you can't at least maintain your own style guidelines... I mean what does a maintainer do?
> You'll notice I didn't say "reformat code when standards change."
> This
> is a distributed project and it doesn't scale to have the code
> continually in flux -- reformatting creates conflicts in other
> contributors patches, which then consume all of a lieutenant's time
> if
> they were to try to fix up each one for them. It's much better to
> push
> push that work out to the edge of the network -- put another way,
> "many
> hands make light work." Fixing existing formatting problems is
> acceptable in a patch series if one is working in the area, but it is
> rare that a series devoted solely to that kind of cleanup gets in.
Because the volunteers don't want to do lowly bitch work.
I'm not a volunteer. I was paid to write the CMAC patch since our project supports it [amongst other modes including AH-GMAC...]. But that said we don't work on kernel time. I don't bend my schedule around the whims of some moderator who is too damn lazy to do a bit of lowly work that they think is beneath them.
> The coding styles have evolved over time, and are different for
> different areas of the kernel -- for example, most of the kernel
> wants
> '/*' on its own line for a multi-line comment, but under net/ it
> should
> not. You should try to match the style in your area -- claiming that
> the
> cryto/ code does it one way is unlikely to sway opinion in net/.
> Different lieutenants, different opinions.
Which of course is asinine.
> As for the existing style issues in net/ipv4/ah4.c you posted, no,
> your
> patch adding a feature in ah4 would probably not been rejected
> because
> of the existing issues, though if one was in the middle of your
> changes,
> it is possible that your would be asked to fix it. As for the issues
> checkpatch.pl found, they are in many cases legitimate, but are also
> the
> exceptions -- there are plenty of counter-examples in that file
> showing
> the preferred style.
Except that my CMAC content came almost exclusively from XCBC which was already in the kernel. Since you guys did reject the patch on the grounds of coding style why would I assume my AH patches would be any different?
Tom
^ permalink raw reply
* [RFC:] struct net_device_ops: Add function pointer to fill device specific ndisc information
From: Stephan Gatzka @ 2013-01-20 17:52 UTC (permalink / raw)
To: linux1394-devel, netdev; +Cc: yoshfuji, stefanr, davem
I've implemented IPv6 over firewire. Right now I'm facing the problem
that the corresponding RFC3146 requires very firewire specific
information sent during neighborhood discovery.
There was already a discussion on the linux1394 mailing list
(http://sourceforge.net/mailarchive/message.php?msg_id=30342089 and
http://sourceforge.net/mailarchive/forum.php?thread_name=50E4A3E5.8080304%40gmail.com&forum_name=linux1394-devel)
During that discussion all participants agreed that it makes no sense to
introduce a dependency between the ndisc code and the firewire net driver.
So the most appealing option seems to be to introduce on more callback
routine either in struct net_device or struct net_device_ops:
int (*ndo_fill_llao)(void *llao);
Because I'm not so familiar with the structure of the whole network
infrastructure in Linux, I need some advice if struct net_device or
struct net_device_ops is the right place.
Maybe it's worth to generalize this and do the same for IPv4/ARP because
right now the ARP packets are mangled in the firewire net driver.
Nevertheless, I've to admit that right now it seems that only IPv6 over
firewire requires such a callback routine.
Regards,
Stephan
^ permalink raw reply
* Re: [PATCH net-next V6 02/14] bridge: Add vlan filtering infrastructure
From: Vlad Yasevich @ 2013-01-20 17:59 UTC (permalink / raw)
To: Michał Mirosław
Cc: netdev, bridge, davem, shemminger, mst, shmulik.ladkani
In-Reply-To: <CAHXqBFKg6Vp5=dprDYKON2LmshhX+nSPF7KRHiQmOS68PrQq-A@mail.gmail.com>
On 01/17/2013 08:57 PM, Michał Mirosław wrote:
> 2013/1/16 Vlad Yasevich <vyasevic@redhat.com>:
> [...]
>> --- /dev/null
>> +++ b/net/bridge/br_vlan.c
> [...]
>> +struct net_port_vlan *nbp_vlan_find(const struct net_port_vlans *v, u16 vid)
>> +{
>> + struct net_port_vlan *pve;
>> +
>> + /* Must be done either in rcu critical section or with RTNL held */
>> + WARN_ON_ONCE(!rcu_read_lock_held() && !rtnl_is_locked());
>> +
>> + list_for_each_entry_rcu(pve, &v->vlan_list, list) {
>> + if (pve->vid == vid)
>> + return pve;
>> + }
>> +
>> + return NULL;
>> +}
>
> This looks expensive - it's O(n) with n = number of configured VLANs on a port.
> And this is called for every packet. The bridge already has a hash of VLAN
> structures found by br_vlan_find(). You could add a second bitmap there
> (eg. ingres_ports[]) and check port's bit instead of walking the list.
> You would use a bit more memory (64 bytes minus the removed list-head)
> per configured VLAN but save some cycles in hot path.
>
Technically wouldn't even need another bitmap as an existing membership
bitmap would cover this case. I did some profiling and the list is
faster for 3 vlans per port. Hash is faster for more then 3 vlans.
I can easily switch to hash if that is what others think.
-vlad
> Best Regards,
> Michał Mirosław
>
^ permalink raw reply
* Re: IPsec AH use of ahash
From: David Dillow @ 2013-01-20 18:11 UTC (permalink / raw)
To: Tom St Denis; +Cc: linux-kernel, netdev
In-Reply-To: <1361595885.93211.1358703620681.JavaMail.root@elliptictech.com>
On Sun, 2013-01-20 at 12:40 -0500, Tom St Denis wrote:
> > On Sun, 2013-01-20 at 10:07 -0500, Tom St Denis wrote:
> > > In all likelihood I will submit a revised CMAC patch but it'll take
> > > time before I can get business hours to work on it. So instead of
> > > having a maintainer just touch it up we're all going to lose out
> > > because of pride?
> >
> > Yes -- but it would seem to be yours that presents the problem.
>
> Not really. Again, *I* have this content. You do not.
You mean the content at http://lkml.org/lkml/2012/12/11/369 ?
Funny how posting to an mailing list works...
> And the only reason you don't is because someone with pull-request
> authority is too lazy [or apathetic] to make minor cosmetic changes
> and get the change pushed through the process.
Someone like the author?
> I was hoping to have a constructive conversation in which the
> maintainers could come clean about the complete hypocrisy of claiming
> that the source is the reference [since there is no documentation for
> anything in the kernel] and then saying you can't actually read the
> source when you want to contribute.
A conversation where you have already decided the outcome -- that's a
speech, not a dialogue.
> Furthermore I don't think you get how the business side of this works.
I've been doing both kernel and proprietary development for close to two
decades, so you might be surprised. I see it the other way around -- you
don't see how the open source part works, and are expecting to
substitute your proprietary process -- where crap code that "works" is
preferred, to hell with trying to maintain consistency, maintainability,
or even readability.
I've worked on those code bases, and the kernel is a breath of fresh air
in comparison.
> I was tasked with getting CMAC to work with IPsec. I did. I sent the
> patch off to the LKML. My boss tasked me with other work (totally
> unrelated to IPsec) since in their eyes the project was done [we do
> after all have CMAC support].
>
> To then go back to my boss and say I need another couple of hours to
> "clean" it, re-test it in our IPsec lab and then resubmit it in hopes
> that the "maintainers" don't find some other reason to reject it is
> totally unprofessional and unproductive.
You sold your boss a bill of goods, then -- that may fly in your shop,
but that doesn't work here. It is well known that there is likely to be
several rounds of review, and this isn't a new thing.
> > You don't think the maintainers are maintaining if they don't clean
> > up
> > after every random contributor -- fine, call them lieutenants then.
> > Their job is to guide the new development towards the goals for the
> > system, review patches, and develop new features.
>
> Know this. Being a software developer involves doing a lot of lowly
> work that most people aren't excited by. You guys already don't
> comment/document your code.
I do know this, and I also know that the kernel is much better
documented than the most of the closed source code bases I've looked at
in my career.
> I'm not a volunteer. I was paid to write the CMAC patch since our
> project supports it [amongst other modes including AH-GMAC...]. But
> that said we don't work on kernel time. I don't bend my schedule
> around the whims of some moderator who is too damn lazy to do a bit of
> lowly work that they think is beneath them.
You think davem is lazy? Gruff, sure, but lazy? Well, thank you for
illuminating your grasp of reality.
> > As for the existing style issues in net/ipv4/ah4.c you posted, no,
> > your patch adding a feature in ah4 would probably not been rejected
> > because of the existing issues, though if one was in the middle of your
> > changes, it is possible that your would be asked to fix it.
> Except that my CMAC content came almost exclusively from XCBC which
> was already in the kernel. Since you guys did reject the patch on the
> grounds of coding style why would I assume my AH patches would be any
> different?
There's a difference between patching an existing file, and introducing
a new one. Should you have introduced a new file based on AH, I suspect
it would also need some cleaning up.
Anyways, I'll stop feeding the troll now. You've made it quite clear
that no one is going to convince you, and calling the maintainers lazy
and hypocrites isn't going to convince them.
^ permalink raw reply
* Re: IPsec AH use of ahash
From: Tom St Denis @ 2013-01-20 18:47 UTC (permalink / raw)
To: David Dillow; +Cc: linux-kernel, netdev
In-Reply-To: <1358705471.2494.51.camel@obelisk.thedillows.org>
----- Original Message -----
> From: "David Dillow" <dave@thedillows.org>
> To: "Tom St Denis" <tstdenis@elliptictech.com>
> Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org
> Sent: Sunday, 20 January, 2013 1:11:11 PM
> Subject: Re: IPsec AH use of ahash
>
> On Sun, 2013-01-20 at 12:40 -0500, Tom St Denis wrote:
> > > On Sun, 2013-01-20 at 10:07 -0500, Tom St Denis wrote:
> > > > In all likelihood I will submit a revised CMAC patch but it'll
> > > > take
> > > > time before I can get business hours to work on it. So instead
> > > > of
> > > > having a maintainer just touch it up we're all going to lose
> > > > out
> > > > because of pride?
> > >
> > > Yes -- but it would seem to be yours that presents the problem.
> >
> > Not really. Again, *I* have this content. You do not.
>
> You mean the content at http://lkml.org/lkml/2012/12/11/369 ?
>
> Funny how posting to an mailing list works...
I posted that in December ... it wasn't till January that I got the first reply back about it failing to meet the coding "standards." Do you honestly expect people to sit on their hands for a month each time simple requests float by?
Furthermore, it's not merged into mainline. So no, you don't have the content.
> > And the only reason you don't is because someone with pull-request
> > authority is too lazy [or apathetic] to make minor cosmetic changes
> > and get the change pushed through the process.
>
> Someone like the author?
You mean the maintainer right? If I have to do all of the work I should get all of the credit.
> > Furthermore I don't think you get how the business side of this
> > works.
>
> I've been doing both kernel and proprietary development for close to
> two
> decades, so you might be surprised. I see it the other way around --
> you
> don't see how the open source part works, and are expecting to
> substitute your proprietary process -- where crap code that "works"
> is
> preferred, to hell with trying to maintain consistency,
> maintainability,
> or even readability.
Um ok that's uncalled for. What part of the fact my CMAC code is like 90% verbatim copied from the XCBC code do you not understand? If you think the CMAC patch was bad then you better call up the authors of the XCBC code and call them out for it too. Stop being hypocritical.
You claimed my code fails to maintain "consistency" ... IT'S COPIED FROM EXISTING CODE. Your complaint makes no sense not to mention it's insulting.
Furthermore, as someone who ran OSS projects (some of which are in millions of devices around the world) I understood to cherish user contributions. Even if they needed polishing up. I did the bitch work of documenting, testing, packaging, development, debugging, and support. It's far far far too common to see OSS developers shy away from the less fun side of software development...
I called out the maintainers here because because not only do they not maintain the integrity of the code for which they claim authority but they are hostile towards outside contributions with ridiculous standards that they themselves don't even adhere to. Code that they work on in the kernel doesn't meet ANY of the standards set forth that I was supposed to magically divine applied here..
> You sold your boss a bill of goods, then -- that may fly in your
> shop,
> but that doesn't work here. It is well known that there is likely to
> be
> several rounds of review, and this isn't a new thing.
Ok sure but why does that involve me re-writing the patch for cosmetic reasons? I agreed that adding the () around the ^/& expressions would be a good idea. However, how is it a good use of my time to re-write the patch (which includes lines of code I didn't write in the first place) for coding styles that are not actually used in the kernel?
I **COPIED** existing code. Yet somehow my code isn't good enough?
> I do know this, and I also know that the kernel is much better
> documented than the most of the closed source code bases I've looked
> at
> in my career.
That's not really an excuse for lack of documentation/comments.
> You think davem is lazy? Gruff, sure, but lazy? Well, thank you for
> illuminating your grasp of reality.
I was a one man shop running open source math/crypto libraries yet I wrote tons of well commented code, 100s of pages of documentation, ran user support, and did all the other things real developers do.
If they can't add missing ()'s (which aren't strictly needed btw) on their own and merge the code into maintain then they're not much of developers now are they?
> There's a difference between patching an existing file, and
> introducing
> a new one. Should you have introduced a new file based on AH, I
> suspect
> it would also need some cleaning up.
Except when the motto is "use the source luke" you shouldn't be surprised when ... people use the source as a template for getting work done. If you guys have such a damn problem with this you should maintain your code so it serves as an example to others. Stop being a hypocrite.
> Anyways, I'll stop feeding the troll now. You've made it quite clear
> that no one is going to convince you, and calling the maintainers
> lazy
> and hypocrites isn't going to convince them.
I'm sure I'll re-write the patch [when I can't say]. I'm not in a position to sell the AH-AEAD work to my bosses because the ROI just won't be there. For now we and our customers will have access to CMAC and nobody else will which is just a crying shame.
I'm not trying to pick a fight I'm trying to point out flaws in the system. That you guys are so immobile on it is a shame. I'd love to sell the idea to my bosses of cleaning up the CryptoAPI/IPsec maybe down the road merging our hardware drivers into mainline but it's not a case I can really make right now. We make due quite fine with a private GPL tree of code but that's the opposite of how this should work.
Tom
^ permalink raw reply
* Re: [RFC:] struct net_device_ops: Add function pointer to fill device specific ndisc information
From: YOSHIFUJI Hideaki @ 2013-01-20 18:47 UTC (permalink / raw)
To: stephan.gatzka; +Cc: linux1394-devel, netdev, David Miller
In-Reply-To: <50FC2EE4.3080705@gmail.com>
Stephan Gatzka wrote:
> I've implemented IPv6 over firewire. Right now I'm facing the problem that the corresponding RFC3146 requires very firewire specific information sent during neighborhood discovery.
>
> There was already a discussion on the linux1394 mailing list (http://sourceforge.net/mailarchive/message.php?msg_id=30342089 and http://sourceforge.net/mailarchive/forum.php?thread_name=50E4A3E5.8080304%40gmail.com&forum_name=linux1394-devel)
>
> During that discussion all participants agreed that it makes no sense to introduce a dependency between the ndisc code and the firewire net driver.
>
> So the most appealing option seems to be to introduce on more callback routine either in struct net_device or struct net_device_ops:
>
> int (*ndo_fill_llao)(void *llao);
>
> Because I'm not so familiar with the structure of the whole network infrastructure in Linux, I need some advice if struct net_device or struct net_device_ops is the right place.
>
> Maybe it's worth to generalize this and do the same for IPv4/ARP because right now the ARP packets are mangled in the firewire net driver.
>
> Nevertheless, I've to admit that right now it seems that only IPv6 over firewire requires such a callback routine.
My current position is to change "mac address" to
struct fwnet_hwaddr {
u8 guid[8];
u8 max_rec;
u8 sspd;
u8 fifo[6];
};
Benefits:
[ARP and NDISC]
- both can be handled in more natural way.
-- You will not need to mangle those packets when
sending/receiving.
-- You do not need to inspect ARP/NDISC packet.
By using netevent notification mechanism, you can
learn peer parameters.
-- IP layer is not required to change very much.
[Multicast]
-it can be handled in more natural way.
-- MCAP (Multicast Channel Allocation Protocol) needs to
know full IP multicast address.
-- We have IP multicast address => "hardware address"
mapping for each L2 type in IP layer.
-- We expect that IP layer can request net_device to
receive multicast stream for some multicast address
via corresponding "hardware address".
-- This means that we need to have "hardware address"
of 128bits (16 octets) or more.
-- By increasing size of dev->dev_addr, we can embedded
full IPv6 address in it.
--yoshfuji
^ permalink raw reply
* Re: [PATCH net-next V6 02/14] bridge: Add vlan filtering infrastructure
From: Stephen Hemminger @ 2013-01-20 19:38 UTC (permalink / raw)
To: vyasevic
Cc: Michał Mirosław, netdev, bridge, davem, shemminger, mst,
shmulik.ladkani
In-Reply-To: <50FC307A.5090003@redhat.com>
On Sun, 20 Jan 2013 12:59:22 -0500
Vlad Yasevich <vyasevic@redhat.com> wrote:
> On 01/17/2013 08:57 PM, Michał Mirosław wrote:
> > 2013/1/16 Vlad Yasevich <vyasevic@redhat.com>:
> > [...]
> >> --- /dev/null
> >> +++ b/net/bridge/br_vlan.c
> > [...]
> >> +struct net_port_vlan *nbp_vlan_find(const struct net_port_vlans *v, u16 vid)
> >> +{
> >> + struct net_port_vlan *pve;
> >> +
> >> + /* Must be done either in rcu critical section or with RTNL held */
> >> + WARN_ON_ONCE(!rcu_read_lock_held() && !rtnl_is_locked());
> >> +
> >> + list_for_each_entry_rcu(pve, &v->vlan_list, list) {
> >> + if (pve->vid == vid)
> >> + return pve;
> >> + }
> >> +
> >> + return NULL;
> >> +}
> >
> > This looks expensive - it's O(n) with n = number of configured VLANs on a port.
> > And this is called for every packet. The bridge already has a hash of VLAN
> > structures found by br_vlan_find(). You could add a second bitmap there
> > (eg. ingres_ports[]) and check port's bit instead of walking the list.
> > You would use a bit more memory (64 bytes minus the removed list-head)
> > per configured VLAN but save some cycles in hot path.
> >
>
> Technically wouldn't even need another bitmap as an existing membership
> bitmap would cover this case. I did some profiling and the list is
> faster for 3 vlans per port. Hash is faster for more then 3 vlans.
>
> I can easily switch to hash if that is what others think.
>
> -vlad
Let's assume the people that really want this feature are using a lot
of vlan's. i.e n = 1000 or so. A bitmap is O(1). Any hash list would
incur a just a big memory penalty for the list head. In other words
a full bitmap is 4096 bits = 512 bytes. If you use hash list,
then the equivalent memory size would be only 64 list heads, therefore
a bitmap is a better choice than a hlist.
^ permalink raw reply
* Re: IPsec AH use of ahash
From: Alan Cox @ 2013-01-20 20:30 UTC (permalink / raw)
To: Tom St Denis, David Dillow; +Cc: linux-kernel, netdev
In-Reply-To: <1361595885.93211.1358703620681.JavaMail.root@elliptictech.com>
Look at it from the kernel end. What happens if your change shows up bugs on another architecture or has a flaw. It works for you now but you plan to dump and run. That's not a viable long term development model for upstream.
The licence allows you to do it, and other parties who care more to pick it up and run with it. Unless someone does however it's just a burden. If nobody wants it upstream enough better it stays out perhaps - if the call is wrong eventually other people will care enoug to share the work. If not you get to pick between doing the extra or re-porting your code to new releases
Alan
^ permalink raw reply
* Re: [patch v3] b43: N-PHY: fix gain in b43_nphy_get_gain_ctl_workaround_ent()
From: Rafał Miłecki @ 2013-01-20 21:01 UTC (permalink / raw)
To: Dan Carpenter
Cc: Stefano Brivio, John W. Linville, linux-wireless, b43-dev, netdev,
linux-kernel, David Laight, kernel-janitors
In-Reply-To: <20130120163130.GA7730@elgon.mountain>
2013/1/20 Dan Carpenter <dan.carpenter@oracle.com>:
> There were no break statements in this switch statement so everything
> used the default settings. Per Walter Harms's suggestion, I've replaced
> the switch statement and done a little cleanup.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> v2: Make additional style fixes as well while we're messing with the
> function.
> v3: Make the array static const int.
Looks fine, thanks Dan
--
Rafał
^ permalink raw reply
* Re: [RFC:] struct net_device_ops: Add function pointer to fill device specific ndisc information
From: Waskiewicz Jr, Peter P @ 2013-01-20 21:22 UTC (permalink / raw)
To: Stephan Gatzka; +Cc: linux1394-devel, netdev, yoshfuji, stefanr, davem
In-Reply-To: <50FC2EE4.3080705@gmail.com>
On Sun, Jan 20, 2013 at 06:52:36PM +0100, Stephan Gatzka wrote:
> I've implemented IPv6 over firewire. Right now I'm facing the
> problem that the corresponding RFC3146 requires very firewire
> specific information sent during neighborhood discovery.
>
> There was already a discussion on the linux1394 mailing list
> (http://sourceforge.net/mailarchive/message.php?msg_id=30342089 and http://sourceforge.net/mailarchive/forum.php?thread_name=50E4A3E5.8080304%40gmail.com&forum_name=linux1394-devel)
>
>
> During that discussion all participants agreed that it makes no
> sense to introduce a dependency between the ndisc code and the
> firewire net driver.
>
> So the most appealing option seems to be to introduce on more
> callback routine either in struct net_device or struct
> net_device_ops:
>
> int (*ndo_fill_llao)(void *llao);
>
> Because I'm not so familiar with the structure of the whole network
> infrastructure in Linux, I need some advice if struct net_device or
> struct net_device_ops is the right place.
>
> Maybe it's worth to generalize this and do the same for IPv4/ARP
> because right now the ARP packets are mangled in the firewire net
> driver.
>
> Nevertheless, I've to admit that right now it seems that only IPv6
> over firewire requires such a callback routine.
I'm no expert on firewire requirements, but if you go down the path
of adding a net_device_ops member, I'd recommend adding a pointer
to your own struct of ops. This would be similar to wireless ops.
Only a suggestion, since you may still need to add more ops later
on, and this way you can contain the inflation to a firewire-specific
struct of function pointers.
Cheers,
-PJ
^ 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