* Re: [patch net-next v3 16/17] bridge: add brport flags to dflt bridge_getlink
From: Jiri Pirko @ 2014-11-26 9:25 UTC (permalink / raw)
To: Thomas Graf
Cc: netdev, davem, nhorman, andy, dborkman, ogerlitz, jesse, pshelar,
azhou, ben, stephen, jeffrey.t.kirsher, vyasevic, xiyou.wangcong,
john.r.fastabend, edumazet, jhs, sfeldma, f.fainelli, roopa,
linville, jasowang, ebiederm, nicolas.dichtel, ryazanov.s.a,
buytenh, aviadr, nbd, alexei.starovoitov, Neil.Jerram, ronye,
simon.horman, alexander.h.duyck, john.ronciak, mleitner, shrijeet,
gospo, bcrl
In-Reply-To: <20141125220753.GD3912@casper.infradead.org>
Tue, Nov 25, 2014 at 11:07:53PM CET, tgraf@suug.ch wrote:
>On 11/25/14 at 11:28am, Jiri Pirko wrote:
>> From: Scott Feldman <sfeldma@gmail.com>
>>
>> To allow brport device to return current brport flags set on port. Add
>> returned flags to nested IFLA_PROTINFO netlink msg built in dflt getlink.
>> With this change, netlink msg returned for bridge_getlink contains the port's
>> offloaded flag settings (the port's SELF settings).
>>
>> Signed-off-by: Scott Feldman <sfeldma@gmail.com>
>> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>
>Minor nit below. Otherwise:
>
>Acked-by: Thomas Graf <tgraf@suug.ch>
>
>> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
>> index bd5e783..91e5368 100644
>> --- a/net/core/rtnetlink.c
>> +++ b/net/core/rtnetlink.c
>> @@ -2687,12 +2687,22 @@ static int rtnl_fdb_dump(struct sk_buff *skb, struct netlink_callback *cb)
>> return skb->len;
>> }
>>
>> +static int brport_nla_put_flag(struct sk_buff *skb, u32 flags, u32 mask,
>> + unsigned int attrnum, unsigned int flag)
>> +{
>> + if (mask & flag)
>> + return nla_put_u8(skb, attrnum, !!(flags & flag));
>
>nla_put_flag()?
No, that is not the same. nla_put_flag works differently. The attr is
either present or not. But in this case, attr is always present and has
value of either 0 or 1.
>
^ permalink raw reply
* [PATCH v2] net: netfilter: Fix undefined reference to nf_nat_redirect_* functions
From: Andreas Ruprecht @ 2014-11-26 9:35 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: Patrick McHardy, Jozsef Kadlecsik, David S. Miller,
netfilter-devel, coreteam, netdev, linux-kernel, Andreas Ruprecht
In-Reply-To: <1416928034-10882-1-git-send-email-rupran@einserver.de>
In a configuration with CONFIG_NFT_NAT and
CONFIG_NETFILTER_XT_TARGET_REDIRECT enabled, undefined references to
nf_nat_redirect_ipv{4,6}() can occur, when the corresponding options
CONFIG_NF_NAT_REDIRECT_IPV4 or CONFIG_NF_NAT_REDIRECT_IPV6 are not
enabled.
net/built-in.o: In function `redirect_tg4':
xt_REDIRECT.c:(.text+0x6d001): undefined reference to `nf_nat_redirect_ipv4'
net/built-in.o: In function `redirect_tg6':
xt_REDIRECT.c:(.text+0x6d021): undefined reference to `nf_nat_redirect_ipv6'
This is because the file xt_REDIRECT.c is compiled when
CONFIG_NETFILTER_XT_TARGET_REDIRECT is enabled, which only depends
on CONFIG_NF_NAT. This option is invisible and can only be selected by
other Kconfig options. In this particular case, it is selected by
CONFIG_NFT_NAT.
This patch changes the dependency for CONFIG_NETFILTER_XT_TARGET_REDIRECT
to only make it visible if at least one of
{CONFIG_NF_NAT_REDIRECT_IPV4, CONFIG_NF_NAT_REDIRECT_IPV6} are enabled.
Additionally it is necessary to provide stubs for the
nf_nat_redirect_ipv{4,6} functions in case the header is included but
the corresponding Kconfig feature is not enabled.
Changes:
v2: Correct capitalization for CONFIG_NF_NAT_REDIRECT_IPV4 in comment.
Signed-off-by: Andreas Ruprecht <rupran@einserver.de>
---
include/net/netfilter/ipv4/nf_nat_redirect.h | 14 ++++++++++++++
include/net/netfilter/ipv6/nf_nat_redirect.h | 13 +++++++++++++
net/netfilter/Kconfig | 2 +-
3 files changed, 28 insertions(+), 1 deletion(-)
diff --git a/include/net/netfilter/ipv4/nf_nat_redirect.h b/include/net/netfilter/ipv4/nf_nat_redirect.h
index 19e1df3a0a4d..56a506dd55cc 100644
--- a/include/net/netfilter/ipv4/nf_nat_redirect.h
+++ b/include/net/netfilter/ipv4/nf_nat_redirect.h
@@ -1,9 +1,23 @@
#ifndef _NF_NAT_REDIRECT_IPV4_H_
#define _NF_NAT_REDIRECT_IPV4_H_
+#include <linux/netfilter.h>
+
+#ifdef CONFIG_NF_NAT_REDIRECT_IPV4
unsigned int
nf_nat_redirect_ipv4(struct sk_buff *skb,
const struct nf_nat_ipv4_multi_range_compat *mr,
unsigned int hooknum);
+#else /* CONFIG_NF_NAT_REDIRECT_IPV4 */
+
+unsigned int
+nf_nat_redirect_ipv4(struct sk_buff *skb,
+ const struct nf_nat_ipv4_multi_range_compat *mr,
+ unsigned int hooknum)
+{
+ return NF_ACCEPT;
+}
+#endif /* CONFIG_NF_NAT_REDIRECT_IPV4 */
+
#endif /* _NF_NAT_REDIRECT_IPV4_H_ */
diff --git a/include/net/netfilter/ipv6/nf_nat_redirect.h b/include/net/netfilter/ipv6/nf_nat_redirect.h
index 1ebdffc461cc..4db9351120ec 100644
--- a/include/net/netfilter/ipv6/nf_nat_redirect.h
+++ b/include/net/netfilter/ipv6/nf_nat_redirect.h
@@ -1,8 +1,21 @@
#ifndef _NF_NAT_REDIRECT_IPV6_H_
#define _NF_NAT_REDIRECT_IPV6_H_
+#include <linux/netfilter.h>
+
+#ifdef CONFIG_NF_NAT_REDIRECT_IPV6
unsigned int
nf_nat_redirect_ipv6(struct sk_buff *skb, const struct nf_nat_range *range,
unsigned int hooknum);
+#else /* CONFIG_NF_NAT_REDIRECT_IPV6 */
+
+unsigned int
+nf_nat_redirect_ipv6(struct sk_buff *skb, const struct nf_nat_range *range,
+ unsigned int hooknum)
+{
+ return NF_ACCEPT;
+}
+#endif /* CONFIG_NF_NAT_REDIRECT_IPV6 */
+
#endif /* _NF_NAT_REDIRECT_IPV6_H_ */
diff --git a/net/netfilter/Kconfig b/net/netfilter/Kconfig
index be8db270aa77..0972851cce03 100644
--- a/net/netfilter/Kconfig
+++ b/net/netfilter/Kconfig
@@ -844,7 +844,7 @@ config NETFILTER_XT_TARGET_RATEEST
config NETFILTER_XT_TARGET_REDIRECT
tristate "REDIRECT target support"
- depends on NF_NAT
+ depends on NF_NAT_IPV4 || NF_NAT_IPV6
select NF_NAT_REDIRECT_IPV4 if NF_NAT_IPV4
select NF_NAT_REDIRECT_IPV6 if NF_NAT_IPV6
---help---
--
1.9.1
^ permalink raw reply related
* Re: BCM4313 & brcmsmac & 3.12: only semi-working?
From: Michael Tokarev @ 2014-11-26 9:52 UTC (permalink / raw)
To: Arend van Spriel
Cc: Maximilian Engelhardt, Rafał Miłecki, Seth Forshee,
brcm80211 development, linux-wireless@vger.kernel.org,
Network Development
In-Reply-To: <5471AC29.6040009@broadcom.com>
I'm sorry this took so long - I was AFK during weekend and had
to deal with a huge backlog after that. Now it is all sorted.
23.11.2014 12:43, Arend van Spriel wrote:
> On 19-11-14 22:00, Michael Tokarev wrote:
[]
> Well, it shows tx looks ok, but with download there is not much of that
> going on. At least no large packets. However, I did find some missing
> pieces related to bt-coex. Given that you device is a wifi-bt combo card
> that is likely an issue for you. One of the missing pieces looks in
> sprom for parameters and that is provided by bcma. However, it does not
> seem to extract bt-coex related stuff. So I have attached a patch based
> on 3.18-rc5 for bcma that dumps the sprom contents. Could you sent that
> content to me.
Here we go. I had to replace pr_debug with pr_err - haven't looked yet,
but the thing is that pr_debug isn't even being compiled into the kernel
here, all the messages are not present in the compiled modules.
[ 525.693474] bcma: bcmasprom:
[ 525.693528] bcma:
[ 525.693592] bcma: 2801
[ 525.693613] bcma: 0000
[ 525.693659] bcma: 1795
[ 525.693679] bcma: 103C
[ 525.693725] bcma: 0070
[ 525.693746] bcma: EDBE
[ 525.693791] bcma: 0000
[ 525.693811] bcma: 2BC4
[ 525.693856] bcma: 2A64
[ 525.693877] bcma: 2964
[ 525.693922] bcma:
[ 525.693938] bcma: 2C64
[ 525.693984] bcma: 3CE7
[ 525.694004] bcma: 46FF
[ 525.694049] bcma: 47FF
[ 525.694070] bcma: 0C00
[ 525.694115] bcma: 0820
[ 525.694136] bcma: 0030
[ 525.694181] bcma: 1002
[ 525.694202] bcma: 9F28
[ 525.694247] bcma: 5D44
[ 525.694267] bcma:
[ 525.694329] bcma: 8080
[ 525.694349] bcma: 1D8F
[ 525.694395] bcma: 0032
[ 525.694415] bcma: 0100
[ 525.694461] bcma: DF00
[ 525.694481] bcma: 71F5
[ 525.694526] bcma: 8400
[ 525.694547] bcma: 0083
[ 525.694592] bcma: 8500
[ 525.694613] bcma: 2010
[ 525.694658] bcma:
[ 525.694674] bcma: 0001
[ 525.694719] bcma: 0000
[ 525.694740] bcma: 0000
[ 525.694785] bcma: 0000
[ 525.694805] bcma: 0000
[ 525.694850] bcma: 0000
[ 525.694871] bcma: 0000
[ 525.694916] bcma: 0000
[ 525.694937] bcma: 0000
[ 525.694982] bcma: 0000
[ 525.695002] bcma:
[ 525.695063] bcma: 0000
[ 525.695084] bcma: 0000
[ 525.695129] bcma: 1008
[ 525.695150] bcma: 0305
[ 525.695195] bcma: 0000
[ 525.695215] bcma: 0000
[ 525.695261] bcma: 0000
[ 525.695281] bcma: 0000
[ 525.695326] bcma: 4727
[ 525.695347] bcma: 8000
[ 525.695392] bcma:
[ 525.695409] bcma: 0002
[ 525.695454] bcma: 0000
[ 525.695474] bcma: 1800
[ 525.695520] bcma: 1800
[ 525.695561] bcma: 0000
[ 525.695610] bcma: 0000
[ 525.695631] bcma: 0000
[ 525.695677] bcma: 0000
[ 525.695698] bcma: 0000
[ 525.695746] bcma: 0000
[ 525.695766] bcma:
[ 525.695827] bcma: 0000
[ 525.695848] bcma: 0000
[ 525.695893] bcma: 0000
[ 525.695914] bcma: 0000
[ 525.695961] bcma: 5372
[ 525.695982] bcma: 1107
[ 525.696027] bcma: 2201
[ 525.696048] bcma: 0040
[ 525.696093] bcma: 0884
[ 525.696116] bcma: 0000
[ 525.696161] bcma:
[ 525.696178] bcma: E006
[ 525.696223] bcma: E659
[ 525.696244] bcma: 5F5A
[ 525.696290] bcma: 5856
[ 525.696310] bcma: 0001
[ 525.696356] bcma: FFFF
[ 525.696376] bcma: 83FF
[ 525.696422] bcma: FFFF
[ 525.696443] bcma: 0003
[ 525.696488] bcma: 0202
[ 525.696508] bcma:
[ 525.696570] bcma: FFFF
[ 525.696590] bcma: 0011
[ 525.698381] bcma: 017A
[ 525.698402] bcma: 0000
[ 525.700181] bcma: 0000
[ 525.700202] bcma: 0000
[ 525.701936] bcma: 0000
[ 525.701957] bcma: 0201
[ 525.703650] bcma: 0000
[ 525.703672] bcma: 7800
[ 525.705278] bcma:
[ 525.706808] bcma: 6410
[ 525.708296] bcma: E398
[ 525.708318] bcma: 0008
[ 525.709774] bcma: 0000
[ 525.709796] bcma: 0000
[ 525.711186] bcma: 0000
[ 525.711207] bcma: 0044
[ 525.712532] bcma: 2400
[ 525.712556] bcma: FCF7
[ 525.713867] bcma: 0089
[ 525.713888] bcma:
[ 525.716500] bcma: 0000
[ 525.716524] bcma: 0000
[ 525.717834] bcma: 0000
[ 525.717855] bcma: 0000
[ 525.719195] bcma: 0000
[ 525.719217] bcma: 0000
[ 525.720530] bcma: 0000
[ 525.720551] bcma: 0000
[ 525.721862] bcma: 0000
[ 525.721882] bcma: 0000
[ 525.723176] bcma:
[ 525.724400] bcma: 0000
[ 525.725678] bcma: 0000
[ 525.725699] bcma: 0048
[ 525.726996] bcma: FED2
[ 525.727017] bcma: 15D9
[ 525.728308] bcma: FAC6
[ 525.728329] bcma: 0000
[ 525.729642] bcma: 0000
[ 525.729664] bcma: 0000
[ 525.730958] bcma: 0000
[ 525.730978] bcma:
[ 525.733526] bcma: 0000
[ 525.733549] bcma: 0000
[ 525.734827] bcma: 0000
[ 525.734848] bcma: 0000
[ 525.736155] bcma: 0000
[ 525.736179] bcma: 0000
[ 525.737466] bcma: 0000
[ 525.737487] bcma: 0000
[ 525.738764] bcma: 0000
[ 525.738785] bcma: 0000
[ 525.740049] bcma:
[ 525.741240] bcma: 0000
[ 525.742501] bcma: 0000
[ 525.742522] bcma: 0000
[ 525.743806] bcma: 0000
[ 525.743827] bcma: 0000
[ 525.745111] bcma: 0000
[ 525.745132] bcma: 0000
[ 525.746433] bcma: 0000
[ 525.746456] bcma: 0000
[ 525.747725] bcma: 0000
[ 525.747746] bcma:
[ 525.750251] bcma: 0000
[ 525.750272] bcma: 0000
[ 525.751532] bcma: 0000
[ 525.751553] bcma: 0000
[ 525.752847] bcma: 0000
[ 525.752871] bcma: 0000
[ 525.754150] bcma: 0000
[ 525.754171] bcma: 0000
[ 525.755441] bcma: 0000
[ 525.755462] bcma: 0000
[ 525.756717] bcma:
[ 525.757900] bcma: 0000
[ 525.759148] bcma: 0000
[ 525.759170] bcma: 0000
[ 525.760436] bcma: 0000
[ 525.760457] bcma: 0000
[ 525.761720] bcma: 0000
[ 525.761741] bcma: 0000
[ 525.763023] bcma: 0000
[ 525.763047] bcma: 0000
[ 525.764301] bcma: 0000
[ 525.764321] bcma:
[ 525.766790] bcma: 0000
[ 525.766813] bcma: 1111
[ 525.768057] bcma: 1111
[ 525.768078] bcma: 0000
[ 525.769355] bcma: 0000
[ 525.769378] bcma: 0000
[ 525.770631] bcma: 0000
[ 525.770652] bcma: 0000
[ 525.771898] bcma: 0000
[ 525.771919] bcma: 2222
[ 525.773158] bcma:
[ 525.774325] bcma: 3222
[ 525.775540] bcma: 0000
[ 525.775561] bcma: 0000
[ 525.776816] bcma: 0000
[ 525.776840] bcma: 0000
[ 525.778089] bcma: 0000
[ 525.778110] bcma: 0000
[ 525.779355] bcma: 0000
[ 525.779380] bcma: 0000
[ 525.780600] bcma: 0000
[ 525.780621] bcma:
[ 525.783015] bcma: 0000
[ 525.783038] bcma: 0000
[ 525.784247] bcma: 0000
[ 525.784268] bcma: 0000
[ 525.785500] bcma: 0000
[ 525.785521] bcma: 0000
[ 525.786759] bcma: 0000
[ 525.786781] bcma: 0000
[ 525.787995] bcma: 0000
[ 525.788016] bcma: 0000
[ 525.789217] bcma:
[ 525.790337] bcma: 0000
[ 525.791505] bcma: 0000
[ 525.791525] bcma: 0000
[ 525.792740] bcma: 0000
[ 525.792765] bcma: 0000
[ 525.793965] bcma: 0000
[ 525.793986] bcma: 0000
[ 525.795168] bcma: 0000
[ 525.795189] bcma: 0000
[ 525.796375] bcma: 0000
[ 525.796399] bcma:
[ 525.798682] bcma: 0000
[ 525.798703] bcma: 0000
[ 525.799885] bcma: 0000
[ 525.799906] bcma: 0000
[ 525.801038] bcma: 0000
[ 525.801058] bcma: 0000
[ 525.802177] bcma: 0000
[ 525.802197] bcma: 0000
[ 525.803320] bcma: 0000
[ 525.803340] bcma: 0000
[ 525.804433] bcma:
[ 525.805460] bcma: 0000
[ 525.806547] bcma: 0000
[ 525.806567] bcma: 0000
[ 525.807671] bcma: 0000
[ 525.807691] bcma: 0000
[ 525.808777] bcma: 0000
[ 525.808796] bcma: 0000
[ 525.809899] bcma: 0000
[ 525.809919] bcma: 0000
[ 525.821789] bcma: bus0: Bus registered
Thanks,
/mjt
^ permalink raw reply
* [PATCH net] r8152: drop the tx packet with invalid length
From: Hayes Wang @ 2014-11-26 9:56 UTC (permalink / raw)
To: netdev; +Cc: nic_swsd, linux-kernel, linux-usb, Hayes Wang
Drop the tx packet which is more than the size of agg_buf_sz. When
creating a bridge with the device, we may get the tx packet with
TSO and the length is more than the gso_max_size which is set by
the driver through netif_set_gso_max_size(). Such packets couldn't
be transmitted and should be dropped directly.
Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
drivers/net/usb/r8152.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index c6554c7..ebdaff7 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -1897,6 +1897,15 @@ static netdev_tx_t rtl8152_start_xmit(struct sk_buff *skb,
{
struct r8152 *tp = netdev_priv(netdev);
+ if ((skb->len + sizeof(struct tx_desc)) > agg_buf_sz) {
+ struct net_device_stats *stats = &netdev->stats;
+
+ dev_kfree_skb_any(skb);
+ stats->tx_dropped++;
+ WARN_ON_ONCE(1);
+ return NETDEV_TX_OK;
+ }
+
skb_tx_timestamp(skb);
skb_queue_tail(&tp->tx_queue, skb);
--
1.9.3
^ permalink raw reply related
* Re: [PATCH v2] net: netfilter: Fix undefined reference to nf_nat_redirect_* functions
From: Florian Westphal @ 2014-11-26 10:24 UTC (permalink / raw)
To: Andreas Ruprecht
Cc: Pablo Neira Ayuso, Patrick McHardy, Jozsef Kadlecsik,
David S. Miller, netfilter-devel, coreteam, netdev, linux-kernel
In-Reply-To: <1416994537-1592-1-git-send-email-rupran@einserver.de>
Andreas Ruprecht <rupran@einserver.de> wrote:
> Additionally it is necessary to provide stubs for the
> nf_nat_redirect_ipv{4,6} functions in case the header is included but
> the corresponding Kconfig feature is not enabled.
Hmmm, not following.
Can you elaborate?
Under which circumstances do we have a call to nf_nat_redirect_ipv4()
(i.e., linker error) but can safely do a noop operation instead of the
requested nat redirect...?
^ permalink raw reply
* Re: [patch net-next v3 09/17] bridge: add API to notify bridge driver of learned FBD on offloaded device
From: Jiri Pirko @ 2014-11-26 10:26 UTC (permalink / raw)
To: Scott Feldman
Cc: Thomas Graf, Andy Gospodarek, Netdev, David S. Miller,
nhorman@tuxdriver.com, Andy Gospodarek, dborkman@redhat.com,
ogerlitz@mellanox.com, jesse@nicira.com, pshelar@nicira.com,
azhou@nicira.com, ben@decadent.org.uk, stephen@networkplumber.org,
Kirsher, Jeffrey T, vyasevic@redhat.com, Cong Wang,
Fastabend, John R, Eric Dumazet, Jamal Hadi Salim,
Florian Fainelli, Roopa Prabhu
In-Reply-To: <CAE4R7bApe59sT6G2oORWP=jOOy=h=Da=EQq98fzR1s+ESZOGFQ@mail.gmail.com>
Wed, Nov 26, 2014 at 02:48:04AM CET, sfeldma@gmail.com wrote:
>On Tue, Nov 25, 2014 at 12:36 PM, Thomas Graf <tgraf@suug.ch> wrote:
>> On 11/25/14 at 11:38am, Andy Gospodarek wrote:
>>> On Tue, Nov 25, 2014 at 11:28:40AM +0100, Jiri Pirko wrote:
>>> > From: Scott Feldman <sfeldma@gmail.com>
>>> >
>>> > When the swdev device learns a new mac/vlan on a port, it sends some async
>>> > notification to the driver and the driver installs an FDB in the device.
>>> > To give a holistic system view, the learned mac/vlan should be reflected
>>> > in the bridge's FBD table, so the user, using normal iproute2 cmds, can view
>>> > what is currently learned by the device. This API on the bridge driver gives
>>> > a way for the swdev driver to install an FBD entry in the bridge FBD table.
>>> > (And remove one).
>>> >
>>> > This is equivalent to the device running these cmds:
>>> >
>>> > bridge fdb [add|del] <mac> dev <dev> vid <vlan id> master
>>> >
>>> > This patch needs some extra eyeballs for review, in paricular around the
>>> > locking and contexts.
>>> >
>>> > Signed-off-by: Scott Feldman <sfeldma@gmail.com>
>>> > Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>>
>> I like the simplicity of this. That said, given we'll have multiple
>> users of swdev including OVS, shouldn't this be a notifier or a
>> callback that depends on who is controlling the device?
>
>I like the idea. When the switch port joins Linux bridge or OVS
>datapath, a callback is registered with the driver. That way the
>driver doesn't really care if the port is a bridge member or an OVS
>vport in a datapath. It's just passing up the FDB entry
>(port/mac/vlan) details to the container device. Can we hold this
>idea until this patchset sticks? I think once OVS support comes back
>into the swdev model would be the time to address this.
Yep, I agree this is a good idea and I also vote for implemeting this as
a follow-up. Thanks.
>
>>
>>> > + spin_lock(&br->hash_lock);
>>> (Since you asked to check locking...)
>>>
>>> Most of the other fdb_add/delete/insert/update calls take this with
>>> spin_lock_bh. Did you try this with lockdep enabled just to see if that
>>> is needed here? I suspect that anytime br->hash_lock is taken it will
>>> need to be with softirqs disabled from this point forward.
>>
>> At least br_fdb_update() seems to be called from BH context so I would
>> agree and argue the lock in br_fdb_cleanup() and br_fdb_update() need a
>> fix too. I'll send a patch.
^ permalink raw reply
* Re: [PATCH v2] net: netfilter: Fix undefined reference to nf_nat_redirect_* functions
From: Andreas Ruprecht @ 2014-11-26 10:33 UTC (permalink / raw)
To: Florian Westphal
Cc: Pablo Neira Ayuso, Patrick McHardy, Jozsef Kadlecsik,
David S. Miller, netfilter-devel, coreteam, netdev, linux-kernel
In-Reply-To: <20141126102402.GA24801@breakpoint.cc>
Sure.
When the file is compiled, i.e. CONFIG_NETFILTER_XT_TARGET_REDIRECT is
selected, all headers will be included and all functions inside the file
will be compiled, regardless of other Kconfig options.
This means redirect_tg6 and redirect_tg4 will be compiled (which doesn't
necessarily mean they will be _called_) but the linker needs to resolve
nf_nat_redirect_ipv4() due to the compilation of the redirect_tg4()
function.
nf_nat_redirect_ip4() is defined in
net/ipv4/netfilter/nf_nat_redirect_ipv4.c but this file is only included
into the build when CONFIG_NF_NAT_REDIRECT_IPV4 is enabled.
Now when a kernel config enables CONFIG_NETFILTER_XT_TARGET_REDIRECT but
_not_ CONFIG_NF_NAT_REDIRECT_IPV4, the declaration of
nf_nat_redirect_ipv4() from the header
<net/netfilter/ipv4/nf_nat_redirect.h> will have no definition (i.e., no
implementation), causing the linker to report an "undefined reference".
Same logic goes for nf_nat_redirect_ipv6().
Hope this helps,
Andreas
On 26.11.2014 11:24, Florian Westphal wrote:
> Andreas Ruprecht <rupran@einserver.de> wrote:
>> Additionally it is necessary to provide stubs for the
>> nf_nat_redirect_ipv{4,6} functions in case the header is included but
>> the corresponding Kconfig feature is not enabled.
>
> Hmmm, not following.
>
> Can you elaborate?
>
> Under which circumstances do we have a call to nf_nat_redirect_ipv4()
> (i.e., linker error) but can safely do a noop operation instead of the
> requested nat redirect...?
>
^ permalink raw reply
* Re: [patch net-next v3 16/17] bridge: add brport flags to dflt bridge_getlink
From: Thomas Graf @ 2014-11-26 10:48 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, nhorman, andy, dborkman, ogerlitz, jesse, pshelar,
azhou, ben, stephen, jeffrey.t.kirsher, vyasevic, xiyou.wangcong,
john.r.fastabend, edumazet, jhs, sfeldma, f.fainelli, roopa,
linville, jasowang, ebiederm, nicolas.dichtel, ryazanov.s.a,
buytenh, aviadr, nbd, alexei.starovoitov, Neil.Jerram, ronye,
simon.horman, alexander.h.duyck, john.ronciak, mleitner, shrijeet,
gospo, bcrl
In-Reply-To: <20141126092512.GE1875@nanopsycho.orion>
On 11/26/14 at 10:25am, Jiri Pirko wrote:
> Tue, Nov 25, 2014 at 11:07:53PM CET, tgraf@suug.ch wrote:
> >> +static int brport_nla_put_flag(struct sk_buff *skb, u32 flags, u32 mask,
> >> + unsigned int attrnum, unsigned int flag)
> >> +{
> >> + if (mask & flag)
> >> + return nla_put_u8(skb, attrnum, !!(flags & flag));
> >
> >nla_put_flag()?
>
> No, that is not the same. nla_put_flag works differently. The attr is
> either present or not. But in this case, attr is always present and has
> value of either 0 or 1.
So this reports the driver capabilities through this interface as
well. Cool. Wasn't obvious to me before but that justifies the waste.
^ permalink raw reply
* Re: [patch net-next v3 17/17] rocker: add ndo_bridge_setlnk/getlink support for learning policy
From: Thomas Graf @ 2014-11-26 11:07 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, nhorman, andy, dborkman, ogerlitz, jesse, pshelar,
azhou, ben, stephen, jeffrey.t.kirsher, vyasevic, xiyou.wangcong,
john.r.fastabend, edumazet, jhs, sfeldma, f.fainelli, roopa,
linville, jasowang, ebiederm, nicolas.dichtel, ryazanov.s.a,
buytenh, aviadr, nbd, alexei.starovoitov, Neil.Jerram, ronye,
simon.horman, alexander.h.duyck, john.ronciak, mleitner, shrijeet,
gospo, bcrl
In-Reply-To: <1416911328-10979-18-git-send-email-jiri@resnulli.us>
On 11/25/14 at 11:28am, Jiri Pirko wrote:
> @@ -3657,6 +3693,64 @@ skip:
> return idx;
> }
>
> +static int rocker_port_bridge_setlink(struct net_device *dev,
> + struct nlmsghdr *nlh)
> +{
> + struct rocker_port *rocker_port = netdev_priv(dev);
> + struct nlattr *protinfo;
> + struct nlattr *afspec;
> + struct nlattr *attr;
> + u16 mode;
> + int err;
> +
> + protinfo = nlmsg_find_attr(nlh, sizeof(struct ifinfomsg),
> + IFLA_PROTINFO);
> + afspec = nlmsg_find_attr(nlh, sizeof(struct ifinfomsg), IFLA_AF_SPEC);
> +
> + if (afspec) {
> + attr = nla_find_nested(afspec, IFLA_BRIDGE_MODE);
> + if (attr) {
> + mode = nla_get_u16(attr);
> + if (mode != BRIDGE_MODE_SWDEV)
> + return -EINVAL;
> + }
> + }
The Netlink message is completely unverified at this point. All
rtnl_bridge_setlink() does is verify that msgsize >= ifinfomsg.
All of the drivers but br_setlink() need fixing in this regard.
^ permalink raw reply
* Re: [PATCH v2] net: netfilter: Fix undefined reference to nf_nat_redirect_* functions
From: Pablo Neira Ayuso @ 2014-11-26 11:24 UTC (permalink / raw)
To: Andreas Ruprecht
Cc: Florian Westphal, Patrick McHardy, Jozsef Kadlecsik,
David S. Miller, netfilter-devel, coreteam, netdev, linux-kernel
In-Reply-To: <5475AC6F.9010902@einserver.de>
On Wed, Nov 26, 2014 at 11:33:19AM +0100, Andreas Ruprecht wrote:
> Sure.
>
> When the file is compiled, i.e. CONFIG_NETFILTER_XT_TARGET_REDIRECT is
> selected, all headers will be included and all functions inside the file
> will be compiled, regardless of other Kconfig options.
>
> This means redirect_tg6 and redirect_tg4 will be compiled (which doesn't
> necessarily mean they will be _called_) but the linker needs to resolve
> nf_nat_redirect_ipv4() due to the compilation of the redirect_tg4()
> function.
>
> nf_nat_redirect_ip4() is defined in
> net/ipv4/netfilter/nf_nat_redirect_ipv4.c but this file is only included
> into the build when CONFIG_NF_NAT_REDIRECT_IPV4 is enabled.
>
> Now when a kernel config enables CONFIG_NETFILTER_XT_TARGET_REDIRECT but
> _not_ CONFIG_NF_NAT_REDIRECT_IPV4, the declaration of
> nf_nat_redirect_ipv4() from the header
> <net/netfilter/ipv4/nf_nat_redirect.h> will have no definition (i.e., no
> implementation), causing the linker to report an "undefined reference".
>
> Same logic goes for nf_nat_redirect_ipv6().
I'd suggest alternatives to resolve this problem:
1) Split xt_REDIRECT into ipt_REDIRECT and ip6t_REDIRECT, so we
restore the state of how this was back in 2012. The main motivation
behind that change was to reduce memory consumption by combining both
modules. In other modules, these combinations have been causing us
problems specifically when IPv6 symbols are used and it's not that
clean since IPv6 specific code remains there unused in the module even
if CONFIG_IPV6=n.
2) Merge nf_nat_redirect_ipv4 and nf_nat_redirect_ipv6 into
nf_nat_redirect, so we inconditionally build IPv6 redirect code, thus
xt_REDIRECT always finds the IPv6 symbol that needs even if it doesn't
use it.
3) Add #ifdef to xt_REDIRECT.c to make IPv6 specific code, this should
be a simple and small patch, but it results in #ifdef pollution.
Comments?
Thanks.
^ permalink raw reply
* Re: [PATCH v2] net: netfilter: Fix undefined reference to nf_nat_redirect_* functions
From: Florian Westphal @ 2014-11-26 11:26 UTC (permalink / raw)
To: Andreas Ruprecht
Cc: Florian Westphal, Pablo Neira Ayuso, Patrick McHardy,
Jozsef Kadlecsik, David S. Miller, netfilter-devel, coreteam,
netdev, linux-kernel
In-Reply-To: <5475AC6F.9010902@einserver.de>
Andreas Ruprecht <rupran@einserver.de> wrote:
> When the file is compiled, i.e. CONFIG_NETFILTER_XT_TARGET_REDIRECT is
> selected, all headers will be included and all functions inside the file
> will be compiled, regardless of other Kconfig options.
>
> This means redirect_tg6 and redirect_tg4 will be compiled (which doesn't
> necessarily mean they will be _called_) but the linker needs to resolve
> nf_nat_redirect_ipv4() due to the compilation of the redirect_tg4()
> function.
So the problem is eg.
CONFIG_NETFILTER_XT_TARGET_REDIRECT=y
CONFIG_NF_NAT_IPV4=n
which yields
undefined reference to `nf_nat_redirect_ipv4'
adding stub fixes this; we will still register a netfilter target for
ipv4 redirect, but it cannot be called ever since that target is
resticted to 'nat' table, which doesn't exist for ipv4 in the above
config.
To me it seem cleaner to put
IS_ENABLED(NF_NAT_REDIRECT_IPV4)
into xt_REDIRECT.c instead of building not-working-but-never-called
ipv4 redirect.
Pablo, if you disagree I am fine with the patch, although
the stubs should get a 'static inline' prefix to not cause sym clash
in case we ever gain another call site.
^ permalink raw reply
* Re: [patch net-next v3 17/17] rocker: add ndo_bridge_setlnk/getlink support for learning policy
From: Jiri Pirko @ 2014-11-26 11:27 UTC (permalink / raw)
To: Thomas Graf
Cc: netdev, davem, nhorman, andy, dborkman, ogerlitz, jesse, pshelar,
azhou, ben, stephen, jeffrey.t.kirsher, vyasevic, xiyou.wangcong,
john.r.fastabend, edumazet, jhs, sfeldma, f.fainelli, roopa,
linville, jasowang, ebiederm, nicolas.dichtel, ryazanov.s.a,
buytenh, aviadr, nbd, alexei.starovoitov, Neil.Jerram, ronye,
simon.horman, alexander.h.duyck, john.ronciak, mleitner, shrijeet,
gospo, bcrl
In-Reply-To: <20141126110709.GC4321@casper.infradead.org>
Wed, Nov 26, 2014 at 12:07:09PM CET, tgraf@suug.ch wrote:
>On 11/25/14 at 11:28am, Jiri Pirko wrote:
>> @@ -3657,6 +3693,64 @@ skip:
>> return idx;
>> }
>>
>> +static int rocker_port_bridge_setlink(struct net_device *dev,
>> + struct nlmsghdr *nlh)
>> +{
>> + struct rocker_port *rocker_port = netdev_priv(dev);
>> + struct nlattr *protinfo;
>> + struct nlattr *afspec;
>> + struct nlattr *attr;
>> + u16 mode;
>> + int err;
>> +
>> + protinfo = nlmsg_find_attr(nlh, sizeof(struct ifinfomsg),
>> + IFLA_PROTINFO);
>> + afspec = nlmsg_find_attr(nlh, sizeof(struct ifinfomsg), IFLA_AF_SPEC);
>> +
>> + if (afspec) {
>> + attr = nla_find_nested(afspec, IFLA_BRIDGE_MODE);
>> + if (attr) {
>> + mode = nla_get_u16(attr);
>> + if (mode != BRIDGE_MODE_SWDEV)
>> + return -EINVAL;
>> + }
>> + }
>
>The Netlink message is completely unverified at this point. All
>rtnl_bridge_setlink() does is verify that msgsize >= ifinfomsg.
>All of the drivers but br_setlink() need fixing in this regard.
I believe that we should fix this for all drivers in a follow-up patch.
^ permalink raw reply
* Re: [patch net-next v3 02/17] net: make vid as a parameter for ndo_fdb_add/ndo_fdb_del
From: Jamal Hadi Salim @ 2014-11-26 11:28 UTC (permalink / raw)
To: Scott Feldman
Cc: John Fastabend, Jiri Pirko, Netdev, David S. Miller,
nhorman@tuxdriver.com, Andy Gospodarek, Thomas Graf,
dborkman@redhat.com, ogerlitz@mellanox.com, jesse@nicira.com,
pshelar@nicira.com, azhou@nicira.com, ben@decadent.org.uk,
stephen@networkplumber.org, Kirsher, Jeffrey T,
vyasevic@redhat.com, Cong Wang, Eric Dumazet, Florian Fainelli,
Roopa Prabhu, John Linville
In-Reply-To: <CAE4R7bA2VPZ3tm2MrsXAm4XfNX0TguT=vUma-3HrZSu98hBGyA@mail.gmail.com>
On 11/25/14 22:59, Scott Feldman wrote:
> On Tue, Nov 25, 2014 at 5:19 PM, Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>> On 11/25/14 21:36, Scott Feldman wrote:
>>
>>>> Ok, guess i am gonna have to go stare at the code some more.
>>>> I thought we returned one of the error codes?
>>>> A bitmask would work for a single entry - because you have two
>>>> options add to h/ware and/or s/ware. So response is easy to encode.
>>>> But if i have 1000 and they are sparsely populated (think an indexed
>>>> table and i have indices 1, 23, 45, etc), then a bitmask would be
>>>> hard to use.
>>>
>>>
>>> I'm confused by this discussion.
>>
>>
>> This is about the policy which states "install as many as you can, dont
>> worry about failures". In such a case, how do you tell user space back
>> "oh, btw you know your request #1, #23, and 45 went ok, but nothing else
>> worked". A simple return code wont work. You could return a code to
>> say "some worked". At which case user space could dump and find out only
>> #1, #23 and #45 worked.
>
> You request for what? That's my confusion.
Scott, you are gonna make do this all over again?;->
The summary is there are three possible policies that could be
identified by the user asking for a kernel operation.
One use case example was to send a bunch of (for example)
create/updates and request that the kernel should not abort on a
failure of a single one but to keep going and create/update as many
as possible. Is that part clear? I know it is not what you do,
but there are use cases for that (Read John's response).
Now assuming someone wants this and some entries failed;
how do you tell user space back what was actually updated vs not?
You could return a code which says "partial success".
Forget whether the table is keyed or indexed but if you wanted
to return more detailed info you would return an array/vector of some
sort with status code per entry. Something netlink cant do.
Is that a better description?
> Are you trying to install
> FDB entry into both SW and HW at same time?
What is wrong with installing on both hardware and software? The
point was to identify what kind of policies could be requested by
the user; but even for the bridge why is it bad that i ask for
both master&self?
It is something I can do today with none of these patches.
> And then do a bunch in a
> batch? I'm saying use MASTER for SW and SELF for HW in two steps,
But that would be enforcing your policy on me.
> if
> you want FDB entry installed in both Sw and HW. Check your return
> code each step. Batch all to HW first, then batch all that PASSED to
> SW. I don't even know really why you're trying to install to both HW
> and SW. Install it to HW and be done. fdb_dump will set HW entries
> via SELF.
>
First off: bad performance, but your call to do it that way
(just please please dont enforce it on me;->)
Lets take the hardware batching you mentioned above and see if
i can help to clarify in the third policy choice (continue-on-failure).
Lets say you have a keyed table such as the fdb table is.
You send 10 entries to be created/added in hardware. #3 and #5 failed
because you made a mistake and sent them with the same key. #9 and #10
failed because the hardware doesnt have any more space.
we didnt stop and go back for #3 and #5 because the user told
us to continue and do the rest when we fail. And s/he did that because
she wanted to put as many entries in hardware as possible without
necessarily needing to know how much space exists.
> Ah, Jamal, look again at patches 13-17/17 in last v3 set. That was a
> big steaming snickerdoodle just for you! Now you can push policy
> knobs down to port driver and or bridge to fine tune what ever you
> want. You'll find knobs for learning, flooding, learning sync to hw,
> etc. I thought you even ACKed some of these.
I think it almost there.
What you are missing is the policy decision to only sync when i
say so. Having an ndo_ops is a necessity but i dont want the driver
to decide for me just because it can ;->
Telling hardware to learn is instructing it to self update its entries
based on source lookup failure. That is distinctly different from
telling to sync to the kernel. So if you add that knob we are in good
shape.
cheers,
jamal
> a) above knob is 14/17
> patch, b) above is using existing learning knob on bridge, c) above I
> don't get...no point in syncing that direction.
>
^ permalink raw reply
* Re: [patch net-next v3 17/17] rocker: add ndo_bridge_setlnk/getlink support for learning policy
From: Thomas Graf @ 2014-11-26 11:30 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, nhorman, andy, dborkman, ogerlitz, jesse, pshelar,
azhou, ben, stephen, jeffrey.t.kirsher, vyasevic, xiyou.wangcong,
john.r.fastabend, edumazet, jhs, sfeldma, f.fainelli, roopa,
linville, jasowang, ebiederm, nicolas.dichtel, ryazanov.s.a,
buytenh, aviadr, nbd, alexei.starovoitov, Neil.Jerram, ronye,
simon.horman, alexander.h.duyck, john.ronciak, mleitner, shrijeet,
gospo, bcrl
In-Reply-To: <20141126112718.GK1875@nanopsycho.orion>
On 11/26/14 at 12:27pm, Jiri Pirko wrote:
> Wed, Nov 26, 2014 at 12:07:09PM CET, tgraf@suug.ch wrote:
> >The Netlink message is completely unverified at this point. All
> >rtnl_bridge_setlink() does is verify that msgsize >= ifinfomsg.
> >All of the drivers but br_setlink() need fixing in this regard.
>
> I believe that we should fix this for all drivers in a follow-up patch.
I'm working on this. Will send fixes later today.
^ permalink raw reply
* Re: [PATCH v2] net: netfilter: Fix undefined reference to nf_nat_redirect_* functions
From: Florian Westphal @ 2014-11-26 11:33 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: Andreas Ruprecht, Florian Westphal, Patrick McHardy,
Jozsef Kadlecsik, David S. Miller, netfilter-devel, coreteam,
netdev, linux-kernel
In-Reply-To: <20141126112449.GA4700@salvia>
Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> 2) Merge nf_nat_redirect_ipv4 and nf_nat_redirect_ipv6 into
> nf_nat_redirect, so we inconditionally build IPv6 redirect code, thus
> xt_REDIRECT always finds the IPv6 symbol that needs even if it doesn't
> use it.
Seems this is the best solution since it would also reduce the
kconfig option symbol count.
afaics there is nothing ipv6 specific in nf_nat_redirect_ipv6 so
this should not cause any of the ususal IPV6=m dependency issues.
^ permalink raw reply
* Re: [patch net-next v3 04/17] net: introduce generic switch devices support
From: Jamal Hadi Salim @ 2014-11-26 11:36 UTC (permalink / raw)
To: Scott Feldman
Cc: Thomas Graf, Jiri Pirko, Netdev, David S. Miller,
nhorman@tuxdriver.com, Andy Gospodarek, dborkman@redhat.com,
ogerlitz@mellanox.com, jesse@nicira.com, pshelar@nicira.com,
azhou@nicira.com, ben@decadent.org.uk, stephen@networkplumber.org,
Kirsher, Jeffrey T, vyasevic@redhat.com, Cong Wang,
Fastabend, John R, Eric Dumazet, Florian Fainelli, Roopa Prabhu,
John Linville
In-Reply-To: <CAE4R7bCYwWt5cem5_v8Y0jOSz5ER5_33bMRE2xkTPOqYvM6dUg@mail.gmail.com>
On 11/25/14 23:18, Scott Feldman wrote:
> On Tue, Nov 25, 2014 at 5:33 PM, Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>
> You have a pointer to the kernel driver for that HW?
I wasnt sure if that was a passive aggressive move there to
question what i am claiming?(Only Canadians are allowed to be
passive aggressive Scott). To answer your question, no
code currently littered with vendor SDK unfortunately (as you
would know!).
But hopefully if we get these changes in correctly it would
not be hard to show the driver working fully in the kernel.
There are definetely a few other pieces of hardware that are
making me come back here and invest time and effort in these
long discussions.
> Can you show how
> you're using Linux tc netlink msg in kernel to program HW? I'd like
> to see the in-kernel API.
>
Lets do the L2/port thing first. But yes, I am using Linux tc in
kernel.
cheers,
jamal
^ permalink raw reply
* Re: [patch net-next v3 02/17] net: make vid as a parameter for ndo_fdb_add/ndo_fdb_del
From: Jiri Pirko @ 2014-11-26 11:40 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: Scott Feldman, John Fastabend, Netdev, David S. Miller,
nhorman@tuxdriver.com, Andy Gospodarek, Thomas Graf,
dborkman@redhat.com, ogerlitz@mellanox.com, jesse@nicira.com,
pshelar@nicira.com, azhou@nicira.com, ben@decadent.org.uk,
stephen@networkplumber.org, Kirsher, Jeffrey T,
vyasevic@redhat.com, Cong Wang, Eric Dumazet, Florian Fainelli,
Roopa Prabhu, John Linville
In-Reply-To: <5475B952.2080500@mojatatu.com>
Wed, Nov 26, 2014 at 12:28:18PM CET, jhs@mojatatu.com wrote:
>On 11/25/14 22:59, Scott Feldman wrote:
>>On Tue, Nov 25, 2014 at 5:19 PM, Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>>>On 11/25/14 21:36, Scott Feldman wrote:
>
>>>
>>>>>Ok, guess i am gonna have to go stare at the code some more.
>>>>>I thought we returned one of the error codes?
>>>>>A bitmask would work for a single entry - because you have two
>>>>>options add to h/ware and/or s/ware. So response is easy to encode.
>>>>>But if i have 1000 and they are sparsely populated (think an indexed
>>>>>table and i have indices 1, 23, 45, etc), then a bitmask would be
>>>>>hard to use.
>>>>
>>>>
>>>>I'm confused by this discussion.
>>>
>>>
>>>This is about the policy which states "install as many as you can, dont
>>>worry about failures". In such a case, how do you tell user space back
>>>"oh, btw you know your request #1, #23, and 45 went ok, but nothing else
>>>worked". A simple return code wont work. You could return a code to
>>>say "some worked". At which case user space could dump and find out only
>>>#1, #23 and #45 worked.
>>
>>You request for what? That's my confusion.
>
>Scott, you are gonna make do this all over again?;->
>The summary is there are three possible policies that could be
>identified by the user asking for a kernel operation.
>One use case example was to send a bunch of (for example)
>create/updates and request that the kernel should not abort on a
>failure of a single one but to keep going and create/update as many
>as possible. Is that part clear? I know it is not what you do,
>but there are use cases for that (Read John's response).
>Now assuming someone wants this and some entries failed;
>how do you tell user space back what was actually updated vs not?
>You could return a code which says "partial success".
>Forget whether the table is keyed or indexed but if you wanted
>to return more detailed info you would return an array/vector of some
>sort with status code per entry. Something netlink cant do.
>Is that a better description?
Sure this is something that is reasonable to request. But that would
require a major changes to userspace api. At this moment, when we are
using the existing api, I would leave this out for phase 1. Let this be
resolved later as a separate work. Does that make sense?
>
>>Are you trying to install
>>FDB entry into both SW and HW at same time?
>
>
>What is wrong with installing on both hardware and software? The
>point was to identify what kind of policies could be requested by
>the user; but even for the bridge why is it bad that i ask for
>both master&self?
>It is something I can do today with none of these patches.
>
>>And then do a bunch in a
>>batch? I'm saying use MASTER for SW and SELF for HW in two steps,
>
>But that would be enforcing your policy on me.
>
>>if
>>you want FDB entry installed in both Sw and HW. Check your return
>>code each step. Batch all to HW first, then batch all that PASSED to
>>SW. I don't even know really why you're trying to install to both HW
>>and SW. Install it to HW and be done. fdb_dump will set HW entries
>>via SELF.
>>
>
>First off: bad performance, but your call to do it that way
>(just please please dont enforce it on me;->)
>
>Lets take the hardware batching you mentioned above and see if
>i can help to clarify in the third policy choice (continue-on-failure).
>Lets say you have a keyed table such as the fdb table is.
>You send 10 entries to be created/added in hardware. #3 and #5 failed
>because you made a mistake and sent them with the same key. #9 and #10
>failed because the hardware doesnt have any more space.
>we didnt stop and go back for #3 and #5 because the user told
>us to continue and do the rest when we fail. And s/he did that because
>she wanted to put as many entries in hardware as possible without
>necessarily needing to know how much space exists.
>
>
>>Ah, Jamal, look again at patches 13-17/17 in last v3 set. That was a
>>big steaming snickerdoodle just for you! Now you can push policy
>>knobs down to port driver and or bridge to fine tune what ever you
>>want. You'll find knobs for learning, flooding, learning sync to hw,
>>etc. I thought you even ACKed some of these.
>
>I think it almost there.
>What you are missing is the policy decision to only sync when i
>say so. Having an ndo_ops is a necessity but i dont want the driver
>to decide for me just because it can ;->
>Telling hardware to learn is instructing it to self update its entries
>based on source lookup failure. That is distinctly different from
>telling to sync to the kernel. So if you add that knob we are in good
>shape.
>
>cheers,
>jamal
>
>>a) above knob is 14/17
>>patch, b) above is using existing learning knob on bridge, c) above I
>>don't get...no point in syncing that direction.
>>
>
^ permalink raw reply
* Re: [patch net-next v3 17/17] rocker: add ndo_bridge_setlnk/getlink support for learning policy
From: Jiri Pirko @ 2014-11-26 11:42 UTC (permalink / raw)
To: Thomas Graf
Cc: netdev, davem, nhorman, andy, dborkman, ogerlitz, jesse, pshelar,
azhou, ben, stephen, jeffrey.t.kirsher, vyasevic, xiyou.wangcong,
john.r.fastabend, edumazet, jhs, sfeldma, f.fainelli, roopa,
linville, jasowang, ebiederm, nicolas.dichtel, ryazanov.s.a,
buytenh, aviadr, nbd, alexei.starovoitov, Neil.Jerram, ronye,
simon.horman, alexander.h.duyck, john.ronciak, mleitner, shrijeet,
gospo, bcrl
In-Reply-To: <20141126113024.GA8410@casper.infradead.org>
Wed, Nov 26, 2014 at 12:30:24PM CET, tgraf@suug.ch wrote:
>On 11/26/14 at 12:27pm, Jiri Pirko wrote:
>> Wed, Nov 26, 2014 at 12:07:09PM CET, tgraf@suug.ch wrote:
>> >The Netlink message is completely unverified at this point. All
>> >rtnl_bridge_setlink() does is verify that msgsize >= ifinfomsg.
>> >All of the drivers but br_setlink() need fixing in this regard.
>>
>> I believe that we should fix this for all drivers in a follow-up patch.
>
>I'm working on this. Will send fixes later today.
Feel free to fix rocker as well. I'll take your patch into my queue.
^ permalink raw reply
* Re: [patch net-next v3 02/17] net: make vid as a parameter for ndo_fdb_add/ndo_fdb_del
From: Jamal Hadi Salim @ 2014-11-26 11:54 UTC (permalink / raw)
To: Jiri Pirko
Cc: Scott Feldman, John Fastabend, Netdev, David S. Miller,
nhorman@tuxdriver.com, Andy Gospodarek, Thomas Graf,
dborkman@redhat.com, ogerlitz@mellanox.com, jesse@nicira.com,
pshelar@nicira.com, azhou@nicira.com, ben@decadent.org.uk,
stephen@networkplumber.org, Kirsher, Jeffrey T,
vyasevic@redhat.com, Cong Wang, Eric Dumazet, Florian Fainelli,
Roopa Prabhu, John Linville
In-Reply-To: <20141126114035.GM1875@nanopsycho.orion>
On 11/26/14 06:40, Jiri Pirko wrote:
> Wed, Nov 26, 2014 at 12:28:18PM CET, jhs@mojatatu.com wrote:
>> Scott, you are gonna make do this all over again?;->
>> The summary is there are three possible policies that could be
>> identified by the user asking for a kernel operation.
>> One use case example was to send a bunch of (for example)
>> create/updates and request that the kernel should not abort on a
>> failure of a single one but to keep going and create/update as many
>> as possible. Is that part clear? I know it is not what you do,
>> but there are use cases for that (Read John's response).
>> Now assuming someone wants this and some entries failed;
>> how do you tell user space back what was actually updated vs not?
>> You could return a code which says "partial success".
>> Forget whether the table is keyed or indexed but if you wanted
>> to return more detailed info you would return an array/vector of some
>> sort with status code per entry. Something netlink cant do.
>> Is that a better description?
>
> Sure this is something that is reasonable to request. But that would
> require a major changes to userspace api. At this moment, when we are
> using the existing api, I would leave this out for phase 1. Let this be
> resolved later as a separate work. Does that make sense?
>
I think these are just discussions so we know where we are going.
I ACKed the patch already but added that we should consider these
policies. Scott take note.
The default behavior should be maintained whatever the new policies are.
The vectoring is going to be a harder thing to get right. It can be done
but long shot probably.
For user->kernel policy description, that is easy; we need 2 bits
from somewhere; probably same namespace as software/hardware.
cheers,
jamal
^ permalink raw reply
* Re: [patch net-next v3 02/17] net: make vid as a parameter for ndo_fdb_add/ndo_fdb_del
From: Jamal Hadi Salim @ 2014-11-26 12:06 UTC (permalink / raw)
To: Jiri Pirko
Cc: Scott Feldman, John Fastabend, Netdev, David S. Miller,
nhorman@tuxdriver.com, Andy Gospodarek, Thomas Graf,
dborkman@redhat.com, ogerlitz@mellanox.com, jesse@nicira.com,
pshelar@nicira.com, azhou@nicira.com, ben@decadent.org.uk,
stephen@networkplumber.org, Kirsher, Jeffrey T,
vyasevic@redhat.com, Cong Wang, Eric Dumazet, Florian Fainelli,
Roopa Prabhu, John Linville
In-Reply-To: <5475BF72.1060606@mojatatu.com>
On 11/26/14 06:54, Jamal Hadi Salim wrote:
> On 11/26/14 06:40, Jiri Pirko wrote:
>> Sure this is something that is reasonable to request. But that would
>> require a major changes to userspace api. At this moment, when we are
>> using the existing api, I would leave this out for phase 1. Let this be
>> resolved later as a separate work. Does that make sense?
>>
>
> I think these are just discussions so we know where we are going.
> I ACKed the patch already but added that we should consider these
> policies. Scott take note.
>
In case i wasnt clear - yes, the patch as is fine ;->
cheers,
jamal
^ permalink raw reply
* Re: [patch net-next v3 13/17] bridge: move private brport flags to if_bridge.h so port drivers can use flags
From: Jiri Pirko @ 2014-11-26 12:12 UTC (permalink / raw)
To: Thomas Graf
Cc: netdev, davem, nhorman, andy, dborkman, ogerlitz, jesse, pshelar,
azhou, ben, stephen, jeffrey.t.kirsher, vyasevic, xiyou.wangcong,
john.r.fastabend, edumazet, jhs, sfeldma, f.fainelli, roopa,
linville, jasowang, ebiederm, nicolas.dichtel, ryazanov.s.a,
buytenh, aviadr, nbd, alexei.starovoitov, Neil.Jerram, ronye,
simon.horman, alexander.h.duyck, john.ronciak, mleitner, shrijeet,
gospo, bcrl
In-Reply-To: <20141125224856.GI3912@casper.infradead.org>
Tue, Nov 25, 2014 at 11:48:56PM CET, tgraf@suug.ch wrote:
>On 11/25/14 at 11:28am, Jiri Pirko wrote:
>> From: Scott Feldman <sfeldma@gmail.com>
>>
>> Signed-off-by: Scott Feldman <sfeldma@gmail.com>
>> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>
>Could be ported to BIT() if you respin.
Done, thanks.
^ permalink raw reply
* Re: [PATCH v4 0/7] kernel tinification: optionally compile out splice family of syscalls (splice, vmsplice, tee and sendfile)
From: One Thousand Gnomes @ 2014-11-26 12:19 UTC (permalink / raw)
To: David Miller
Cc: josh, rdunlap, pieter, alexander.h.duyck, viro, ast, akpm, beber,
catalina.mocanu, dborkman, edumazet, ebiederm, fabf, fuse-devel,
geert, hughd, iulia.manda21, JBeulich, bfields, jlayton,
linux-api, linux-fsdevel, linux-kernel, linux-nfs, mcgrof,
mattst88, mgorman, mst, miklos, netdev, oleg, Paul.Durrant,
paulmck, pefoley2, tgraf, therbert, trond.myklebust, willemb,
xiaoguangrong
In-Reply-To: <20141125.140441.401150380839514113.davem@davemloft.net>
On Tue, 25 Nov 2014 14:04:41 -0500 (EST)
David Miller <davem@davemloft.net> wrote:
> From: josh@joshtriplett.org
> Date: Tue, 25 Nov 2014 10:53:10 -0800
>
> > It's not a "slippery slope"; it's been our standard practice for ages.
>
> We've never put an entire class of generic system calls behind
> a config option.
Try running an original MCC Linux binary and C lib on a current kernel
We've put *entire binary formats* behind a config option. We've put older
syscalls behind it, we've put sysfs behind it, sysctl behind it, the
older microcode interfaces behind it, ISA bus as a concept behind
options. VDSO, IPC, even 32bit support ... the list goes on and on.
I'd say those were far more generic on the whole than splice/sendfile.
Alan
^ permalink raw reply
* Re: [PATCH v4 20/42] virtio_net: pass vi around
From: Cornelia Huck @ 2014-11-26 12:35 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: rusty, netdev, linux-kernel, virtualization, pbonzini,
David Miller
In-Reply-To: <1416933600-21398-21-git-send-email-mst@redhat.com>
On Tue, 25 Nov 2014 18:42:49 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> Too many places poke at [rs]q->vq->vdev->priv just to get
> the the vi structure. Let's just pass the pointer around: seems
s/the the/the/
> cleaner, and might even be faster.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> drivers/net/virtio_net.c | 38 ++++++++++++++++++++------------------
> 1 file changed, 20 insertions(+), 18 deletions(-)
>
Looks reasonable.
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
^ permalink raw reply
* [PATCH 0/5 net] bridge: Fix missing Netlink message validations
From: Thomas Graf @ 2014-11-26 12:42 UTC (permalink / raw)
To: davem; +Cc: stephen, netdev
Adds various missing length checks in the bridging code for Netlink
messages and corresponding attributes provided by user space.
Thomas Graf (5):
bridge: Validate IFLA_BRIDGE_FLAGS attribute length
net: Validate IFLA_BRIDGE_MODE attribute length
net: Check for presence of IFLA_AF_SPEC
bridge: Add missing policy entry for IFLA_BRPORT_FAST_LEAVE
bridge: Sanitize IFLA_EXT_MASK for AF_BRIDGE:RTM_GETLINK
drivers/net/ethernet/emulex/benet/be_main.c | 5 +++++
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 5 +++++
net/bridge/br_netlink.c | 1 +
net/core/rtnetlink.c | 23 ++++++++++++++++++-----
4 files changed, 29 insertions(+), 5 deletions(-)
--
1.9.3
^ permalink raw reply
* [PATCH 1/5] bridge: Validate IFLA_BRIDGE_FLAGS attribute length
From: Thomas Graf @ 2014-11-26 12:42 UTC (permalink / raw)
To: davem; +Cc: stephen, netdev, Vlad Yasevich
In-Reply-To: <cover.1417005245.git.tgraf@suug.ch>
Payload is currently accessed blindly and may exceed valid message
boundaries.
Fixes: 407af3299 ("bridge: Add netlink interface to configure vlans on bridge ports")
Cc: Vlad Yasevich <vyasevic@redhat.com>
Signed-off-by: Thomas Graf <tgraf@suug.ch>
---
net/core/rtnetlink.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index a688268..5a853f8 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -2798,6 +2798,9 @@ static int rtnl_bridge_setlink(struct sk_buff *skb, struct nlmsghdr *nlh)
if (br_spec) {
nla_for_each_nested(attr, br_spec, rem) {
if (nla_type(attr) == IFLA_BRIDGE_FLAGS) {
+ if (nla_len(attr) < sizeof(flags))
+ return -EINVAL;
+
have_flags = true;
flags = nla_get_u16(attr);
break;
@@ -2868,6 +2871,9 @@ static int rtnl_bridge_dellink(struct sk_buff *skb, struct nlmsghdr *nlh)
if (br_spec) {
nla_for_each_nested(attr, br_spec, rem) {
if (nla_type(attr) == IFLA_BRIDGE_FLAGS) {
+ if (nla_len(attr) < sizeof(flags))
+ return -EINVAL;
+
have_flags = true;
flags = nla_get_u16(attr);
break;
--
1.9.3
^ 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