* Re: [patch net-next v2 07/10] bridge: call netdev_sw_port_stp_update when bridge port STP status changes
From: Roopa Prabhu @ 2014-11-10 15:59 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: Jiri Pirko, netdev, davem, nhorman, andy, tgraf, dborkman,
ogerlitz, jesse, pshelar, azhou, ben, stephen, jeffrey.t.kirsher,
vyasevic, xiyou.wangcong, john.r.fastabend, edumazet, sfeldma,
f.fainelli, 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: <5460B989.8030404@mojatatu.com>
On 11/10/14, 5:11 AM, Jamal Hadi Salim wrote:
> On 11/09/14 05:51, Jiri Pirko wrote:
>> From: Scott Feldman <sfeldma@gmail.com>
>>
>> To notify switch driver of change in STP state of bridge port, add new
>> .ndo op and provide swdev wrapper func to call ndo op. Use it in bridge
>> code then.
>>
>> Signed-off-by: Scott Feldman <sfeldma@gmail.com>
>> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>> ---
>> include/linux/netdevice.h | 6 ++++++
>> include/net/switchdev.h | 6 ++++++
>> net/bridge/br_netlink.c | 2 ++
>> net/bridge/br_stp.c | 4 ++++
>> net/bridge/br_stp_if.c | 3 +++
>> net/bridge/br_stp_timer.c | 2 ++
>> net/switchdev/switchdev.c | 19 +++++++++++++++++++
>> 7 files changed, 42 insertions(+)
>>
>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>> index 116a19d..35f21a95 100644
>> --- a/include/linux/netdevice.h
>> +++ b/include/linux/netdevice.h
>> @@ -1033,6 +1033,10 @@ typedef u16 (*select_queue_fallback_t)(struct
>> net_device *dev,
>> * const unsigned char *addr,
>> * u16 vid);
>> * Called to delete a fdb from switch device port.
>> + *
>> + * int (*ndo_sw_port_stp_update)(struct net_device *dev, u8 state);
>> + * Called to notify switch device port of bridge port STP
>> + * state change.
>
> You are unconditionally calling
> netdev_sw_port_stp_update(p->dev, p->state);
> Again issue is policy. Could you make this work the same
> way the fdb_add e.g user intent of whether i want to turn
> a port in hardware and/or software to disabled/learning/etc
> is reflected?
>
> btw: does _sw_ stand for switch? why not _hw_ ?
> Could we have one ndo for all flags instead of individual ones.
I agree. There is the bridge port state and a bunch of bridge port
flags. A generic ndo will be good.
>
> I know the current user space code uses u8 as a bitflag; but
> maybe we can introduce a new u32 flag bitmask that has all the
> flags set for backward compat? I can count about a total of 10.
^ permalink raw reply
* Re: [RFC PATCH net-next] net: Convert LIMIT_NETDEBUG to net_dbg_ratelimited
From: Joe Perches @ 2014-11-10 15:26 UTC (permalink / raw)
To: nicolas.dichtel; +Cc: David Miller, netdev, linux-kernel, Remi Denis-Courmont
In-Reply-To: <5460CB43.4050008@6wind.com>
On Mon, 2014-11-10 at 15:27 +0100, Nicolas Dichtel wrote:
> Le 09/11/2014 20:17, Joe Perches a écrit :
> > Use the more common dynamic_debug capable net_dbg_ratelimited
> > and remove the LIMIT_NETDEBUG macro.
> >
> > This may have some negative impact on messages that were
> > emitted at KERN_INFO that are not not enabled at all unless
> > DEBUG is defined or dynamic_debug is enabled. Even so,
> > these messages are now _not_ emitted by default.
> >
> > This eliminates the use of the net_msg_warn sysctl
> > "/proc/sys/net/core/warnings".
[]
> > diff --git a/include/net/sock.h b/include/net/sock.h
[]
> > @@ -2276,13 +2276,7 @@ bool sk_ns_capable(const struct sock *sk,
> > bool sk_capable(const struct sock *sk, int cap);
> > bool sk_net_capable(const struct sock *sk, int cap);
> >
> > -/*
> > - * Enable debug/info messages
> > - */
> > -extern int net_msg_warn;
> > -#define LIMIT_NETDEBUG(fmt, args...) \
> > - do { if (net_msg_warn && net_ratelimit()) printk(fmt,##args); } while(0)
> > -
> > +extern int net_msg_warn; /* Unused, but still a sysctl */
> Why not removing this variable from this header and from net/core/utils.c?
> Just declaring a static variable in net/core/sysctl_net_core.c should be enough.
> Am I missing something?
No. It's reasonable to remove its EXPORT_SYMBOL use too.
First let's see if there are any objections to the removal.
^ permalink raw reply
* Re: BUG in xennet_make_frags with paged skb data
From: David Vrabel @ 2014-11-10 14:41 UTC (permalink / raw)
To: netdev, David S. Miller, Eric Dumazet, Konrad Rzeszutek Wilk,
Boris Ostrovsky, Stefan Bader, Jay Vosburgh, linux-kernel,
xen-devel
In-Reply-To: <20141110143517.GA74005@ubuntu-hedt>
On 10/11/14 14:35, Seth Forshee wrote:
> On Fri, Nov 07, 2014 at 10:44:15AM +0000, David Vrabel wrote:
>> On 06/11/14 21:49, Seth Forshee wrote:
>>> We've had several reports of hitting the following BUG_ON in
>>> xennet_make_frags with 3.2 and 3.13 kernels (I'm currently awaiting
>>> results of testing with 3.17):
>>>
>>> /* Grant backend access to each skb fragment page. */
>>> for (i = 0; i < frags; i++) {
>>> skb_frag_t *frag = skb_shinfo(skb)->frags + i;
>>> struct page *page = skb_frag_page(frag);
>>>
>>> len = skb_frag_size(frag);
>>> offset = frag->page_offset;
>>>
>>> /* Data must not cross a page boundary. */
>>> BUG_ON(len + offset > PAGE_SIZE<<compound_order(page));
>>>
>>> When this happens the page in question is a "middle" page in a compound
>>> page (i.e. it's a tail page but not the last tail page), and the data is
>>> fully contained within the compound page. The data does however cross
>>> the hardware page boundary, and since compound_order evaluates to 0 for
>>> tail pages the check fails.
>>>
>>> In going over this I've been unable to determine whether the BUG_ON in
>>> xennet_make_frags is incorrect or the paged skb data is wrong. I can't
>>> find that it's documented anywhere, and the networking code itself is a
>>> bit ambiguous when it comes to compound pages. On the one hand
>>> __skb_fill_page_desc specifically handles adding tail pages as paged
>>> data, but on the other hand skb_copy_bits kmaps frag->page.p which could
>>> fail with data that extends into another page.
>>
>> netfront will safely handle this case so you can remove this BUG_ON()
>> (and the one later on). But it would be better to find out were these
>> funny-looking skbs are coming from and (if necessary) fixing the bug there.
>
> There still seems to be disagreement about whether the "funny" skb is
> valid though - you imply it isn't, but Eric says it is. I've been trying
> to track down where these skbs originate, and so far I've determined
> that they come from a socket spliced to a pipe spliced to a socket. It
> looks like the particular page/offset/len tuple originates at least as
> far back as the first socket, as the tuple is simply copied from an skb
> into the pipe and from the pipe into the final skb.
Apologies for the lack of clarity. I meant either: a) fix the producer
if these skbs are invalid; or b) remove the BUG_ON()s. Since Eric says
these are actually valid skbs, please do option (b).
i.e., remove both BUG_ON()s.
David
^ permalink raw reply
* Re: BUG in xennet_make_frags with paged skb data
From: Seth Forshee @ 2014-11-10 14:35 UTC (permalink / raw)
To: David Vrabel
Cc: netdev, David S. Miller, Eric Dumazet, Konrad Rzeszutek Wilk,
Boris Ostrovsky, Stefan Bader, Jay Vosburgh, linux-kernel,
xen-devel, seth.forshee
In-Reply-To: <545CA27F.4070400@citrix.com>
On Fri, Nov 07, 2014 at 10:44:15AM +0000, David Vrabel wrote:
> On 06/11/14 21:49, Seth Forshee wrote:
> > We've had several reports of hitting the following BUG_ON in
> > xennet_make_frags with 3.2 and 3.13 kernels (I'm currently awaiting
> > results of testing with 3.17):
> >
> > /* Grant backend access to each skb fragment page. */
> > for (i = 0; i < frags; i++) {
> > skb_frag_t *frag = skb_shinfo(skb)->frags + i;
> > struct page *page = skb_frag_page(frag);
> >
> > len = skb_frag_size(frag);
> > offset = frag->page_offset;
> >
> > /* Data must not cross a page boundary. */
> > BUG_ON(len + offset > PAGE_SIZE<<compound_order(page));
> >
> > When this happens the page in question is a "middle" page in a compound
> > page (i.e. it's a tail page but not the last tail page), and the data is
> > fully contained within the compound page. The data does however cross
> > the hardware page boundary, and since compound_order evaluates to 0 for
> > tail pages the check fails.
> >
> > In going over this I've been unable to determine whether the BUG_ON in
> > xennet_make_frags is incorrect or the paged skb data is wrong. I can't
> > find that it's documented anywhere, and the networking code itself is a
> > bit ambiguous when it comes to compound pages. On the one hand
> > __skb_fill_page_desc specifically handles adding tail pages as paged
> > data, but on the other hand skb_copy_bits kmaps frag->page.p which could
> > fail with data that extends into another page.
>
> netfront will safely handle this case so you can remove this BUG_ON()
> (and the one later on). But it would be better to find out were these
> funny-looking skbs are coming from and (if necessary) fixing the bug there.
There still seems to be disagreement about whether the "funny" skb is
valid though - you imply it isn't, but Eric says it is. I've been trying
to track down where these skbs originate, and so far I've determined
that they come from a socket spliced to a pipe spliced to a socket. It
looks like the particular page/offset/len tuple originates at least as
far back as the first socket, as the tuple is simply copied from an skb
into the pipe and from the pipe into the final skb.
Anyway, it looks like at minimum it should be safe to change the first
BUG_ON to assert that the data is fully within the compound page as
Stefan suggested, then remove the second BUG_ON entirely. This is the
path I plan to pursue unless someone objects.
Thanks,
Seth
^ permalink raw reply
* Re: [PATCH] stmmac: split to core library and probe drivers
From: Giuseppe CAVALLARO @ 2014-11-10 14:28 UTC (permalink / raw)
To: Andy Shevchenko, netdev, Kweh Hock Leong, David S . Miller,
Vince Bridgers
In-Reply-To: <1415615939-6671-1-git-send-email-andriy.shevchenko@linux.intel.com>
Hi Andy
On 11/10/2014 11:38 AM, Andy Shevchenko wrote:
> Instead of registering the platform and PCI drivers in one module let's move
> necessary bits to where it belongs. During this procedure we convert the module
> registration part to use module_*_driver() macros which makes code simplier.
>
>>From now on the driver consists three parts: core library, PCI, and platform
> drivers.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
patch looks fine and net-next.
Acked-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> ---
> drivers/net/ethernet/stmicro/stmmac/Kconfig | 4 +-
> drivers/net/ethernet/stmicro/stmmac/Makefile | 15 +++---
> drivers/net/ethernet/stmicro/stmmac/stmmac.h | 61 ----------------------
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 36 ++-----------
> drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 4 +-
> .../net/ethernet/stmicro/stmmac/stmmac_platform.c | 13 ++---
> 6 files changed, 25 insertions(+), 108 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> index 33b85ba..7d3af19 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
> +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> @@ -14,7 +14,7 @@ config STMMAC_ETH
> if STMMAC_ETH
>
> config STMMAC_PLATFORM
> - bool "STMMAC Platform bus support"
> + tristate "STMMAC Platform bus support"
> depends on STMMAC_ETH
> default y
> ---help---
> @@ -27,7 +27,7 @@ config STMMAC_PLATFORM
> If unsure, say N.
>
> config STMMAC_PCI
> - bool "STMMAC PCI bus support"
> + tristate "STMMAC PCI bus support"
> depends on STMMAC_ETH && PCI
> ---help---
> This is to select the Synopsys DWMAC available on PCI devices,
> diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
> index 034da70..ac4d562 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/Makefile
> +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
> @@ -1,9 +1,12 @@
> obj-$(CONFIG_STMMAC_ETH) += stmmac.o
> -stmmac-$(CONFIG_STMMAC_PCI) += stmmac_pci.o
> -stmmac-$(CONFIG_STMMAC_PLATFORM) += stmmac_platform.o dwmac-meson.o \
> - dwmac-sunxi.o dwmac-sti.o \
> - dwmac-socfpga.o
> stmmac-objs:= stmmac_main.o stmmac_ethtool.o stmmac_mdio.o ring_mode.o \
> - chain_mode.o dwmac_lib.o dwmac1000_core.o dwmac1000_dma.o \
> - dwmac100_core.o dwmac100_dma.o enh_desc.o norm_desc.o \
> + chain_mode.o dwmac_lib.o dwmac1000_core.o dwmac1000_dma.o \
> + dwmac100_core.o dwmac100_dma.o enh_desc.o norm_desc.o \
> mmc_core.o stmmac_hwtstamp.o stmmac_ptp.o $(stmmac-y)
> +
> +obj-$(CONFIG_STMMAC_PLATFORM) += stmmac-platform.o
> +stmmac-platform-objs:= stmmac_platform.o dwmac-meson.o dwmac-sunxi.o \
> + dwmac-sti.o dwmac-socfpga.o
> +
> +obj-$(CONFIG_STMMAC_PCI) += stmmac-pci.o
> +stmmac-pci-objs:= stmmac_pci.o
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> index bd75ee8..c0a3919 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> @@ -134,65 +134,4 @@ struct stmmac_priv *stmmac_dvr_probe(struct device *device,
> void stmmac_disable_eee_mode(struct stmmac_priv *priv);
> bool stmmac_eee_init(struct stmmac_priv *priv);
>
> -#ifdef CONFIG_STMMAC_PLATFORM
> -extern struct platform_driver stmmac_pltfr_driver;
> -
> -static inline int stmmac_register_platform(void)
> -{
> - int err;
> -
> - err = platform_driver_register(&stmmac_pltfr_driver);
> - if (err)
> - pr_err("stmmac: failed to register the platform driver\n");
> -
> - return err;
> -}
> -
> -static inline void stmmac_unregister_platform(void)
> -{
> - platform_driver_unregister(&stmmac_pltfr_driver);
> -}
> -#else
> -static inline int stmmac_register_platform(void)
> -{
> - pr_debug("stmmac: do not register the platf driver\n");
> -
> - return 0;
> -}
> -
> -static inline void stmmac_unregister_platform(void)
> -{
> -}
> -#endif /* CONFIG_STMMAC_PLATFORM */
> -
> -#ifdef CONFIG_STMMAC_PCI
> -extern struct pci_driver stmmac_pci_driver;
> -static inline int stmmac_register_pci(void)
> -{
> - int err;
> -
> - err = pci_register_driver(&stmmac_pci_driver);
> - if (err)
> - pr_err("stmmac: failed to register the PCI driver\n");
> -
> - return err;
> -}
> -
> -static inline void stmmac_unregister_pci(void)
> -{
> - pci_unregister_driver(&stmmac_pci_driver);
> -}
> -#else
> -static inline int stmmac_register_pci(void)
> -{
> - pr_debug("stmmac: do not register the PCI driver\n");
> -
> - return 0;
> -}
> -
> -static inline void stmmac_unregister_pci(void)
> -{
> -}
> -#endif /* CONFIG_STMMAC_PCI */
> -
> #endif /* __STMMAC_H__ */
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 53db11b..0f1c146 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -2885,6 +2885,7 @@ error_clk_get:
>
> return ERR_PTR(ret);
> }
> +EXPORT_SYMBOL_GPL(stmmac_dvr_probe);
>
> /**
> * stmmac_dvr_remove
> @@ -2914,8 +2915,8 @@ int stmmac_dvr_remove(struct net_device *ndev)
>
> return 0;
> }
> +EXPORT_SYMBOL_GPL(stmmac_dvr_remove);
>
> -#ifdef CONFIG_PM
> int stmmac_suspend(struct net_device *ndev)
> {
> struct stmmac_priv *priv = netdev_priv(ndev);
> @@ -2957,6 +2958,7 @@ int stmmac_suspend(struct net_device *ndev)
> priv->oldduplex = -1;
> return 0;
> }
> +EXPORT_SYMBOL_GPL(stmmac_suspend);
>
> int stmmac_resume(struct net_device *ndev)
> {
> @@ -3003,37 +3005,7 @@ int stmmac_resume(struct net_device *ndev)
>
> return 0;
> }
> -#endif /* CONFIG_PM */
> -
> -/* Driver can be configured w/ and w/ both PCI and Platf drivers
> - * depending on the configuration selected.
> - */
> -static int __init stmmac_init(void)
> -{
> - int ret;
> -
> - ret = stmmac_register_platform();
> - if (ret)
> - goto err;
> - ret = stmmac_register_pci();
> - if (ret)
> - goto err_pci;
> - return 0;
> -err_pci:
> - stmmac_unregister_platform();
> -err:
> - pr_err("stmmac: driver registration failed\n");
> - return ret;
> -}
> -
> -static void __exit stmmac_exit(void)
> -{
> - stmmac_unregister_platform();
> - stmmac_unregister_pci();
> -}
> -
> -module_init(stmmac_init);
> -module_exit(stmmac_exit);
> +EXPORT_SYMBOL_GPL(stmmac_resume);
>
> #ifndef MODULE
> static int __init stmmac_cmdline_opt(char *str)
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> index 5084699..77a6d68 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> @@ -158,7 +158,7 @@ static const struct pci_device_id stmmac_id_table[] = {
>
> MODULE_DEVICE_TABLE(pci, stmmac_id_table);
>
> -struct pci_driver stmmac_pci_driver = {
> +static struct pci_driver stmmac_pci_driver = {
> .name = STMMAC_RESOURCE_NAME,
> .id_table = stmmac_id_table,
> .probe = stmmac_pci_probe,
> @@ -168,6 +168,8 @@ struct pci_driver stmmac_pci_driver = {
> },
> };
>
> +module_pci_driver(stmmac_pci_driver);
> +
> MODULE_DESCRIPTION("STMMAC 10/100/1000 Ethernet PCI driver");
> MODULE_AUTHOR("Rayagond Kokatanur <rayagond.kokatanur@vayavyalabs.com>");
> MODULE_AUTHOR("Giuseppe Cavallaro <peppe.cavallaro@st.com>");
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> index 9f18401..e22a960 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> @@ -362,7 +362,7 @@ static int stmmac_pltfr_remove(struct platform_device *pdev)
> return ret;
> }
>
> -#ifdef CONFIG_PM
> +#ifdef CONFIG_PM_SLEEP
> static int stmmac_pltfr_suspend(struct device *dev)
> {
> int ret;
> @@ -388,13 +388,12 @@ static int stmmac_pltfr_resume(struct device *dev)
>
> return stmmac_resume(ndev);
> }
> -
> -#endif /* CONFIG_PM */
> +#endif /* CONFIG_PM_SLEEP */
>
> static SIMPLE_DEV_PM_OPS(stmmac_pltfr_pm_ops,
> - stmmac_pltfr_suspend, stmmac_pltfr_resume);
> + stmmac_pltfr_suspend, stmmac_pltfr_resume);
>
> -struct platform_driver stmmac_pltfr_driver = {
> +static struct platform_driver stmmac_pltfr_driver = {
> .probe = stmmac_pltfr_probe,
> .remove = stmmac_pltfr_remove,
> .driver = {
> @@ -402,9 +401,11 @@ struct platform_driver stmmac_pltfr_driver = {
> .owner = THIS_MODULE,
> .pm = &stmmac_pltfr_pm_ops,
> .of_match_table = of_match_ptr(stmmac_dt_ids),
> - },
> + },
> };
>
> +module_platform_driver(stmmac_pltfr_driver);
> +
> MODULE_DESCRIPTION("STMMAC 10/100/1000 Ethernet PLATFORM driver");
> MODULE_AUTHOR("Giuseppe Cavallaro <peppe.cavallaro@st.com>");
> MODULE_LICENSE("GPL");
>
^ permalink raw reply
* Re: [RFC PATCH net-next] net: Convert LIMIT_NETDEBUG to net_dbg_ratelimited
From: Nicolas Dichtel @ 2014-11-10 14:27 UTC (permalink / raw)
To: Joe Perches, David Miller; +Cc: netdev, linux-kernel, Remi Denis-Courmont
In-Reply-To: <1415560642.23530.43.camel@perches.com>
Le 09/11/2014 20:17, Joe Perches a écrit :
> Use the more common dynamic_debug capable net_dbg_ratelimited
> and remove the LIMIT_NETDEBUG macro.
>
> This may have some negative impact on messages that were
> emitted at KERN_INFO that are not not enabled at all unless
> DEBUG is defined or dynamic_debug is enabled. Even so,
> these messages are now _not_ emitted by default.
>
> This eliminates the use of the net_msg_warn sysctl
> "/proc/sys/net/core/warnings".
>
> All messages are still ratelimited.
>
> Some KERN_LEVEL uses are changed to KERN_DEBUG.
>
> Miscellanea:
>
> o Update the sysctl documentation
> o Remove the embedded uses of pr_fmt
> o Coalesce format fragments
> o Realign arguments
>
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
>
> Let me know if you want this consolidate patch broken up
> into multiple patches or any of the messages and the
> macro kept.
>
> Documentation/sysctl/net.txt | 12 ++++++++----
> include/net/sock.h | 8 +-------
> include/net/udplite.h | 6 +++---
> net/ipv4/icmp.c | 8 ++++----
> net/ipv4/inet_fragment.c | 2 +-
> net/ipv4/ip_fragment.c | 3 +--
> net/ipv4/tcp_input.c | 8 ++++----
> net/ipv4/tcp_timer.c | 18 ++++++++++--------
> net/ipv4/udp.c | 30 +++++++++++++++---------------
> net/ipv6/addrconf.c | 6 ++----
> net/ipv6/ah6.c | 7 +++----
> net/ipv6/datagram.c | 4 ++--
> net/ipv6/esp6.c | 4 ++--
> net/ipv6/exthdrs.c | 18 +++++++++---------
> net/ipv6/icmp.c | 15 +++++++--------
> net/ipv6/mip6.c | 11 ++++++-----
> net/ipv6/netfilter.c | 2 +-
> net/ipv6/udp.c | 31 +++++++++++++------------------
> net/phonet/af_phonet.c | 9 +++++----
> net/phonet/pep-gprs.c | 3 +--
> net/phonet/pep.c | 12 ++++++------
> 21 files changed, 104 insertions(+), 113 deletions(-)
>
> diff --git a/Documentation/sysctl/net.txt b/Documentation/sysctl/net.txt
> index 04892b8..46cd03d 100644
> --- a/Documentation/sysctl/net.txt
> +++ b/Documentation/sysctl/net.txt
> @@ -120,10 +120,14 @@ seconds.
> warnings
> --------
>
> -This controls console messages from the networking stack that can occur because
> -of problems on the network like duplicate address or bad checksums. Normally,
> -this should be enabled, but if the problem persists the messages can be
> -disabled.
> +This sysctl is now unused.
> +
> +This was used to control console messages from the networking stack that
> +occur because of problems on the network like duplicate address or bad
> +checksums.
> +
> +These messages are now emitted at KERN_DEBUG and can generally be enabled
> +and controlled by the dynamic_debug facility.
>
> netdev_budget
> -------------
> diff --git a/include/net/sock.h b/include/net/sock.h
> index 6767d75..db363ad 100644
> --- a/include/net/sock.h
> +++ b/include/net/sock.h
> @@ -2276,13 +2276,7 @@ bool sk_ns_capable(const struct sock *sk,
> bool sk_capable(const struct sock *sk, int cap);
> bool sk_net_capable(const struct sock *sk, int cap);
>
> -/*
> - * Enable debug/info messages
> - */
> -extern int net_msg_warn;
> -#define LIMIT_NETDEBUG(fmt, args...) \
> - do { if (net_msg_warn && net_ratelimit()) printk(fmt,##args); } while(0)
> -
> +extern int net_msg_warn; /* Unused, but still a sysctl */
Why not removing this variable from this header and from net/core/utils.c?
Just declaring a static variable in net/core/sysctl_net_core.c should be enough.
Am I missing something?
Nicolas
^ permalink raw reply
* Re: [patch net-next v2 07/10] bridge: call netdev_sw_port_stp_update when bridge port STP status changes
From: Thomas Graf @ 2014-11-10 14:04 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: Jiri Pirko, netdev, davem, nhorman, andy, dborkman, ogerlitz,
jesse, pshelar, azhou, ben, stephen, jeffrey.t.kirsher, vyasevic,
xiyou.wangcong, john.r.fastabend, edumazet, 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: <5460B989.8030404@mojatatu.com>
On 11/10/14 at 08:11am, Jamal Hadi Salim wrote:
> You are unconditionally calling
> netdev_sw_port_stp_update(p->dev, p->state);
> Again issue is policy. Could you make this work the same
> way the fdb_add e.g user intent of whether i want to turn
> a port in hardware and/or software to disabled/learning/etc
> is reflected?
Agreed. Can be added in a next series perhaps?
> btw: does _sw_ stand for switch? why not _hw_ ?
> Could we have one ndo for all flags instead of individual ones.
>
> I know the current user space code uses u8 as a bitflag; but
> maybe we can introduce a new u32 flag bitmask that has all the
> flags set for backward compat? I can count about a total of 10.
I think we can just extend the size of IFLA_BRPORT_STATE, accept
both a u8 and u32, and return a u32 that that is compatible to
existing u8 readers.
^ permalink raw reply
* Re: [patch net-next v2 06/10] bridge: introduce fdb offloading via switchdev
From: Thomas Graf @ 2014-11-10 13:51 UTC (permalink / raw)
To: Jiri Pirko
Cc: Jamal Hadi Salim, netdev, davem, nhorman, andy, dborkman,
ogerlitz, jesse, pshelar, azhou, ben, stephen, jeffrey.t.kirsher,
vyasevic, xiyou.wangcong, john.r.fastabend, edumazet, 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: <20141110081552.GD1850@nanopsycho.orion>
On 11/10/14 at 09:15am, Jiri Pirko wrote:
> There are few problems in re-using this. It is netlink based so for calling
> it from bridge code, we would have to construct netlink message. But
> that could be probably changed.
> As you can see from the list of parameters, this is no longer about fdb (addr,
> vlanid) but this has been extended to something else. See vxlan code for
> what this is used for. I believe that fdb_add/del should be renamed to
> something else, perhaps l2neigh_add/del or something like that.
> The other problem is that fdb_add/del is currently used by various
> drivers for different purpose (adding macs to unicast list).
Can you elaborate a bit on the intended semantic differences between
the existing ndo_fdb_add() and ndo_sw_port_fdb_add()? I'm not sure we
need the sw_ prefix for this specific ndo.
I completely agree that relying on Netlink is wrong because we'll have
in-kernel users of the API but I believe that existing ndo_fdb_add()
implementations in i40e, ixgbe, qlcnic and macvlan could use the new
API you propose.
How about we rename the existing ndo_fdb_add() to ndo_neigh_add() as
you propose and convert vxlan over to it and have all others which don't
even depend on the Netlink attributes being passed in (i40e, ixgbe,
qlcnic, macvlan) use ndo_fdb_add() which would have the behaviour of your
proposed ndo_sw_port_fdb_add()?
^ permalink raw reply
* Re: [patch net-next v2 06/10] bridge: introduce fdb offloading via switchdev
From: Jiri Pirko @ 2014-11-10 13:47 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: netdev, davem, nhorman, andy, tgraf, dborkman, ogerlitz, jesse,
pshelar, azhou, ben, stephen, jeffrey.t.kirsher, vyasevic,
xiyou.wangcong, john.r.fastabend, edumazet, 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: <5460B3E5.7020502@mojatatu.com>
Mon, Nov 10, 2014 at 01:47:33PM CET, jhs@mojatatu.com wrote:
>On 11/10/14 03:15, Jiri Pirko wrote:
>>Mon, Nov 10, 2014 at 04:47:48AM CET, jhs@mojatatu.com wrote:
>>>On 11/09/14 05:51, Jiri Pirko wrote:
>>>>From: Scott Feldman <sfeldma@gmail.com>
>>>>
>
>>Jamal, I believe we discussed this already.
>
>I cant remember how that ended.
>
>>The thing is that current
>>fdb_add/del does not need vlanid and master/self flags, because it
>>already has that (struct nlattr *tb[]). Here is the whole list of
>>parameters to these functions:
>> NDA_DST,
>> NDA_LLADDR,
>> NDA_CACHEINFO,
>> NDA_PROBES,
>> NDA_VLAN,
>> NDA_PORT,
>> NDA_VNI,
>> NDA_IFINDEX,
>> NDA_MASTER,
>>
>>There are few problems in re-using this. It is netlink based so for calling
>>it from bridge code, we would have to construct netlink message. But
>>that could be probably changed.
>
>Trying to understand.
>
>A netlink message for a bridge to add an fdb is targeted at the
>*bridge port*.
>That message has semantic which says "please add this entry
>to the software bridge and/or offloaded hardware".
>If something is targetted at the bridge port, ->ndo_fdb_add()
>is invoked with an internally chewed structure.
>Why would you have to construct a new netlink message to the driver?
Because now, If you would like to pass one of NDA_DST, NDA_LLADDR,
NDA_CACHEINFO, NDA_PROBES, NDA_VLAN, NDA_PORT, NDA_VNI, NDA_IFINDEX,
NDA_MASTER values via ndo_fdb_add/del to the driver, you have to
construct "struct nlattr *tb[]". Preprocessing this tb into struct might
be suitable for some use-case, for some it may not.
>
>
>>As you can see from the list of parameters, this is no longer about fdb (addr,
>>vlanid) but this has been extended to something else.
>
>I am still missing understanding that part.
>Or maybe are you saying that you dont want to pass netlink
>constructs to the driver?
What I try to say is that the naming ndo_fdb_add/del is not accurate
because it is now used for far more than fdb (addr, vlan). See vxlan
code for example.
>
>>See vxlan code for
>>what this is used for. I believe that fdb_add/del should be renamed to
>>something else, perhaps l2neigh_add/del or something like that.
>>The other problem is that fdb_add/del is currently used by various
>>drivers for different purpose (adding macs to unicast list).
>>
>
>Ok, now a small spark ignited in my brain. You did talk about renaming
>things to neighXXX in one of the exchanges. I think this is a separate
>issue from the question of why you cant refactor ndo_fdb_add/del
It can be probably refactored in a way so it fits our fdb offloading
needs. I'm not really sure we would want it. ndo_fdb_* use-case
is dirrerent from what we introduce with ndo_sw_port_fdb_*. The only
similarity is the "fdb" name which in case of ndo_fdb_* is no longer
correct I believe.
>
>The abuse of using this interface for unicast addresses is probably
>driven by the fact some of the hardware probably offloads vlanid 0 or
>something speacial like 4095 to point to the underlying hardware that
>"this belongs to host cpu".
>I am not a fan of it (and have posted in exchanges with Vlad in the
>past).
>
>cheers,
>jamal
^ permalink raw reply
* Re: [patch net-next v2 01/10] net: rename netdev_phys_port_id to more generic name
From: Jamal Hadi Salim @ 2014-11-10 13:20 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, nhorman, andy, tgraf, dborkman, ogerlitz, jesse,
pshelar, azhou, ben, stephen, jeffrey.t.kirsher, vyasevic,
xiyou.wangcong, john.r.fastabend, edumazet, 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: <20141110131622.GC4256@nanopsycho.orion>
On 11/10/14 08:16, Jiri Pirko wrote:
> Jamal, please look at the patch & code. MAX_PHYS_PORT_ID_LEN is in
> include/linux/netdevice.h which is not part of user exported api.
>
I got it - the confusing part was rtnetlink.c was looking for it
as if it was expecting user space to send it.
cheers,
jamal
^ permalink raw reply
* Re: [patch net-next v2 01/10] net: rename netdev_phys_port_id to more generic name
From: Jiri Pirko @ 2014-11-10 13:16 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: netdev, davem, nhorman, andy, tgraf, dborkman, ogerlitz, jesse,
pshelar, azhou, ben, stephen, jeffrey.t.kirsher, vyasevic,
xiyou.wangcong, john.r.fastabend, edumazet, 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: <5460ACC0.3020805@mojatatu.com>
Mon, Nov 10, 2014 at 01:17:04PM CET, jhs@mojatatu.com wrote:
>On 11/10/14 02:43, Jiri Pirko wrote:
>>Mon, Nov 10, 2014 at 04:35:12AM CET, jhs@mojatatu.com wrote:
>
>>I don't see a reason why this would break kabi:
>>
>>-#define MAX_PHYS_PORT_ID_LEN 32
>>+#define MAX_PHYS_ITEM_ID_LEN 32
>>
>
>refer to my response to Dave. Just define MAX_PHYS_PORT_ID_LEN to
>MAX_PHYS_ITEM_ID_LEN so people dont have to change their code
>because a name change.
Jamal, please look at the patch & code. MAX_PHYS_PORT_ID_LEN is in
include/linux/netdevice.h which is not part of user exported api.
>
>cheers,
>jamal
^ permalink raw reply
* Re: [patch net-next v2 00/10] introduce rocker switch driver with hardware accelerated datapath api - phase 1: bridge fdb offload
From: Jiri Pirko @ 2014-11-10 13:12 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: netdev, davem, nhorman, andy, tgraf, dborkman, ogerlitz, jesse,
pshelar, azhou, ben, stephen, jeffrey.t.kirsher, vyasevic,
xiyou.wangcong, john.r.fastabend, edumazet, 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: <5460AC90.9060607@mojatatu.com>
Mon, Nov 10, 2014 at 01:16:16PM CET, jhs@mojatatu.com wrote:
>On 11/10/14 02:23, Jiri Pirko wrote:
>
>>
>>Yes I looked over their patches. Roopas patche's are about new class of
>>device which, as I commented in the cover letter, I left out for now and
>>can be safely added later on.
>>
>>I went over the Ben's work very carefully as well. The patches are very
>>rough, mostly rtl-chip specific. But again, my patchset is a base on
>>which this patches can be build on. I see no issues in that.
>>
>>>At least please get their sign on - this is such an important piece of
>>>new work that you should make sure you get consensus.
>>
>>Since I did not use their code now, I only put sign off of Scott.
>>
>
>Your last comment was "i am going to merge the patches" ;->
>At least send an email explaining your plan to people who have worked
>hard to cooperate with you or say it in the cover letter.
Well I had feedback only from Roopa and we discussed it in following
email thread. I never had any feedback from Ben. I only saw you pasting
link to his git.
There's really nothing else to merge at the moment. I would love to went
over patches and merge them into my tree if anyone sends them.
>
>>>Otherwise we are back to square one and everyone is going their way with
>>>their patches;
>>
>>I do think that we are in sync. I do not see any counter ways. As I
>>said, their work can be added on to the base made of this patchset.
>>
>
>Ok, I hope so. I spoke for myself - it is important for this patches
>you get their sign-on in my opinion.
>
>cheers,
>jamal
>
^ permalink raw reply
* Re: [patch net-next v2 07/10] bridge: call netdev_sw_port_stp_update when bridge port STP status changes
From: Jamal Hadi Salim @ 2014-11-10 13:11 UTC (permalink / raw)
To: Jiri Pirko, netdev
Cc: davem, nhorman, andy, tgraf, dborkman, ogerlitz, jesse, pshelar,
azhou, ben, stephen, jeffrey.t.kirsher, vyasevic, xiyou.wangcong,
john.r.fastabend, edumazet, 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: <1415530280-9190-8-git-send-email-jiri@resnulli.us>
On 11/09/14 05:51, Jiri Pirko wrote:
> From: Scott Feldman <sfeldma@gmail.com>
>
> To notify switch driver of change in STP state of bridge port, add new
> .ndo op and provide swdev wrapper func to call ndo op. Use it in bridge
> code then.
>
> Signed-off-by: Scott Feldman <sfeldma@gmail.com>
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
> ---
> include/linux/netdevice.h | 6 ++++++
> include/net/switchdev.h | 6 ++++++
> net/bridge/br_netlink.c | 2 ++
> net/bridge/br_stp.c | 4 ++++
> net/bridge/br_stp_if.c | 3 +++
> net/bridge/br_stp_timer.c | 2 ++
> net/switchdev/switchdev.c | 19 +++++++++++++++++++
> 7 files changed, 42 insertions(+)
>
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 116a19d..35f21a95 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -1033,6 +1033,10 @@ typedef u16 (*select_queue_fallback_t)(struct net_device *dev,
> * const unsigned char *addr,
> * u16 vid);
> * Called to delete a fdb from switch device port.
> + *
> + * int (*ndo_sw_port_stp_update)(struct net_device *dev, u8 state);
> + * Called to notify switch device port of bridge port STP
> + * state change.
You are unconditionally calling
netdev_sw_port_stp_update(p->dev, p->state);
Again issue is policy. Could you make this work the same
way the fdb_add e.g user intent of whether i want to turn
a port in hardware and/or software to disabled/learning/etc
is reflected?
btw: does _sw_ stand for switch? why not _hw_ ?
Could we have one ndo for all flags instead of individual ones.
I know the current user space code uses u8 as a bitflag; but
maybe we can introduce a new u32 flag bitmask that has all the
flags set for backward compat? I can count about a total of 10.
cheers,
jamal
^ permalink raw reply
* Re: [patch net-next v2 01/10] net: rename netdev_phys_port_id to more generic name
From: Jamal Hadi Salim @ 2014-11-10 12:56 UTC (permalink / raw)
To: Daniel Borkmann
Cc: David Miller, jiri, netdev, nhorman, andy, tgraf, ogerlitz, jesse,
pshelar, azhou, ben, stephen, jeffrey.t.kirsher, vyasevic,
xiyou.wangcong, john.r.fastabend, edumazet, 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: <5460B090.7080106@redhat.com>
On 11/10/14 07:33, Daniel Borkmann wrote:
> $ git grep -n MAX_PHYS_PORT_ID_LEN
> include/linux/netdevice.h:756:#define MAX_PHYS_PORT_ID_LEN 32
> include/linux/netdevice.h:762: unsigned char id[MAX_PHYS_PORT_ID_LEN];
> net/core/rtnetlink.c:871: +
> nla_total_size(MAX_PHYS_PORT_ID_LEN); /* IFLA_PHYS_PORT_ID */
> net/core/rtnetlink.c:1199: [IFLA_PHYS_PORT_ID] = { .type =
> NLA_BINARY, .len = MAX_PHYS_PORT_ID_LEN },
>
> ... and based on commit 66cae9ed6bc4 ("rtnl: export physical port id
> via RT netlink") only exported as read-only.
>
I guess it is *not exported* if no user space code sees it.
If that is the case, I agree that my suggestion is unneeded.
cheers,
jamal
^ permalink raw reply
* Re: [patch net-next v2 06/10] bridge: introduce fdb offloading via switchdev
From: Jamal Hadi Salim @ 2014-11-10 12:47 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, nhorman, andy, tgraf, dborkman, ogerlitz, jesse,
pshelar, azhou, ben, stephen, jeffrey.t.kirsher, vyasevic,
xiyou.wangcong, john.r.fastabend, edumazet, 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: <20141110081552.GD1850@nanopsycho.orion>
On 11/10/14 03:15, Jiri Pirko wrote:
> Mon, Nov 10, 2014 at 04:47:48AM CET, jhs@mojatatu.com wrote:
>> On 11/09/14 05:51, Jiri Pirko wrote:
>>> From: Scott Feldman <sfeldma@gmail.com>
>>>
> Jamal, I believe we discussed this already.
I cant remember how that ended.
> The thing is that current
> fdb_add/del does not need vlanid and master/self flags, because it
> already has that (struct nlattr *tb[]). Here is the whole list of
> parameters to these functions:
> NDA_DST,
> NDA_LLADDR,
> NDA_CACHEINFO,
> NDA_PROBES,
> NDA_VLAN,
> NDA_PORT,
> NDA_VNI,
> NDA_IFINDEX,
> NDA_MASTER,
>
> There are few problems in re-using this. It is netlink based so for calling
> it from bridge code, we would have to construct netlink message. But
> that could be probably changed.
Trying to understand.
A netlink message for a bridge to add an fdb is targeted at the
*bridge port*.
That message has semantic which says "please add this entry
to the software bridge and/or offloaded hardware".
If something is targetted at the bridge port, ->ndo_fdb_add()
is invoked with an internally chewed structure.
Why would you have to construct a new netlink message to the driver?
> As you can see from the list of parameters, this is no longer about fdb (addr,
> vlanid) but this has been extended to something else.
I am still missing understanding that part.
Or maybe are you saying that you dont want to pass netlink
constructs to the driver?
> See vxlan code for
> what this is used for. I believe that fdb_add/del should be renamed to
> something else, perhaps l2neigh_add/del or something like that.
> The other problem is that fdb_add/del is currently used by various
> drivers for different purpose (adding macs to unicast list).
>
Ok, now a small spark ignited in my brain. You did talk about renaming
things to neighXXX in one of the exchanges. I think this is a separate
issue from the question of why you cant refactor ndo_fdb_add/del
The abuse of using this interface for unicast addresses is probably
driven by the fact some of the hardware probably offloads vlanid 0 or
something speacial like 4095 to point to the underlying hardware that
"this belongs to host cpu".
I am not a fan of it (and have posted in exchanges with Vlad in the
past).
cheers,
jamal
^ permalink raw reply
* Re: [patch net-next v2 01/10] net: rename netdev_phys_port_id to more generic name
From: Daniel Borkmann @ 2014-11-10 12:33 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: David Miller, jiri, netdev, nhorman, andy, tgraf, ogerlitz, jesse,
pshelar, azhou, ben, stephen, jeffrey.t.kirsher, vyasevic,
xiyou.wangcong, john.r.fastabend, edumazet, 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: <5460AA45.6000007@mojatatu.com>
On 11/10/2014 01:06 PM, Jamal Hadi Salim wrote:
> On 11/10/14 00:23, David Miller wrote:
>> From: Jamal Hadi Salim <jhs@mojatatu.com>
>> Date: Sun, 09 Nov 2014 22:35:12 -0500
>>
>>> wouldnt this just break an existing ABI? You may need to introduce a
>>> new attribute.
>>
>> He isn't breaking anything Jamal, he's just changing the internal
>> macro name we use for the attribute's maximum length.
>
> It is a _user space visible rename_, how about:
>
> #define MAX_PHYS_ITEM_ID_LEN 32
> #define MAX_PHYS_PORT_ID_LEN MAX_PHYS_ITEM_ID_LEN
>
> I did miss the fact that the size didnt change.
Actually, it's currently not exposed via any uapi header ...
$ git grep -n MAX_PHYS_PORT_ID_LEN
include/linux/netdevice.h:756:#define MAX_PHYS_PORT_ID_LEN 32
include/linux/netdevice.h:762: unsigned char id[MAX_PHYS_PORT_ID_LEN];
net/core/rtnetlink.c:871: + nla_total_size(MAX_PHYS_PORT_ID_LEN); /* IFLA_PHYS_PORT_ID */
net/core/rtnetlink.c:1199: [IFLA_PHYS_PORT_ID] = { .type = NLA_BINARY, .len = MAX_PHYS_PORT_ID_LEN },
... and based on commit 66cae9ed6bc4 ("rtnl: export physical port id
via RT netlink") only exported as read-only.
Best,
Daniel
^ permalink raw reply
* Re: [patch net-next v2 10/10] rocker: implement L2 bridge offloading
From: Jamal Hadi Salim @ 2014-11-10 12:27 UTC (permalink / raw)
To: Scott Feldman
Cc: Jiri Pirko, Netdev, David S. Miller, nhorman, Andy Gospodarek,
Thomas Graf, dborkman, ogerlitz, jesse, pshelar, azhou, ben,
stephen, Kirsher, Jeffrey T, vyasevic, xiyou.wangcong,
Fastabend, John R, edumazet, Florian Fainelli, Roopa Prabhu,
John Linville, jasowang, ebiederm, nicolas.dichtel, ryazanov.s.a,
buytenh, aviadr, nbd, alexei.starovoitov, Neil.Jerram, ronye,
simon.ho
In-Reply-To: <CAE4R7bCdfn6rzvY5J5d_aj=eNKoT00iEphGJMnQOWy=WeLePTw@mail.gmail.com>
On 11/10/14 03:46, Scott Feldman wrote:
>
> IFLA_BRPORT_LEARNING is u8 attr and we're only using lower bit to turn
> learning on/off. Maybe we can use another bit to indicate learning to
> be done in sw or hw. I don't think adding another bit would break
> existing iproute2.
>
> LEARNING_ENABLED (1 << 0)
> LEARNING_HW (1 << 1)
>
> Would this work?
>
Yes to making it a bit. But:
This is not *learning*. You are doing a *sync*.
Those are two different things.
Learning on/off exists today. It signals to the L2 whether you
should learn or not.
I like the way fdb_add/del work with a flag which says
it is the software and/or offloaded version. Please keep that
semantic.
What you are doing above is letting the hardware learn then
syncing to software. You need a different flag there. something
like:
SYNC_HW_FDB (1<<1)
cheers,
jamal
^ permalink raw reply
* Re: [patch net-next v2 01/10] net: rename netdev_phys_port_id to more generic name
From: Jamal Hadi Salim @ 2014-11-10 12:17 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, nhorman, andy, tgraf, dborkman, ogerlitz, jesse,
pshelar, azhou, ben, stephen, jeffrey.t.kirsher, vyasevic,
xiyou.wangcong, john.r.fastabend, edumazet, 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: <20141110074344.GB1850@nanopsycho.orion>
On 11/10/14 02:43, Jiri Pirko wrote:
> Mon, Nov 10, 2014 at 04:35:12AM CET, jhs@mojatatu.com wrote:
> I don't see a reason why this would break kabi:
>
> -#define MAX_PHYS_PORT_ID_LEN 32
> +#define MAX_PHYS_ITEM_ID_LEN 32
>
refer to my response to Dave. Just define MAX_PHYS_PORT_ID_LEN to
MAX_PHYS_ITEM_ID_LEN so people dont have to change their code
because a name change.
cheers,
jamal
^ permalink raw reply
* Re: [patch net-next v2 00/10] introduce rocker switch driver with hardware accelerated datapath api - phase 1: bridge fdb offload
From: Jamal Hadi Salim @ 2014-11-10 12:16 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, nhorman, andy, tgraf, dborkman, ogerlitz, jesse,
pshelar, azhou, ben, stephen, jeffrey.t.kirsher, vyasevic,
xiyou.wangcong, john.r.fastabend, edumazet, 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: <20141110072301.GA1850@nanopsycho.orion>
On 11/10/14 02:23, Jiri Pirko wrote:
>
> Yes I looked over their patches. Roopas patche's are about new class of
> device which, as I commented in the cover letter, I left out for now and
> can be safely added later on.
>
> I went over the Ben's work very carefully as well. The patches are very
> rough, mostly rtl-chip specific. But again, my patchset is a base on
> which this patches can be build on. I see no issues in that.
>
>> At least please get their sign on - this is such an important piece of
>> new work that you should make sure you get consensus.
>
> Since I did not use their code now, I only put sign off of Scott.
>
Your last comment was "i am going to merge the patches" ;->
At least send an email explaining your plan to people who have worked
hard to cooperate with you or say it in the cover letter.
>> Otherwise we are back to square one and everyone is going their way with
>> their patches;
>
> I do think that we are in sync. I do not see any counter ways. As I
> said, their work can be added on to the base made of this patchset.
>
Ok, I hope so. I spoke for myself - it is important for this patches
you get their sign-on in my opinion.
cheers,
jamal
^ permalink raw reply
* Re: [patch net-next v2 01/10] net: rename netdev_phys_port_id to more generic name
From: Jamal Hadi Salim @ 2014-11-10 12:06 UTC (permalink / raw)
To: David Miller
Cc: jiri, netdev, nhorman, andy, tgraf, dborkman, ogerlitz, jesse,
pshelar, azhou, ben, stephen, jeffrey.t.kirsher, vyasevic,
xiyou.wangcong, john.r.fastabend, edumazet, 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: <20141110.002336.70350092543746386.davem@davemloft.net>
On 11/10/14 00:23, David Miller wrote:
> From: Jamal Hadi Salim <jhs@mojatatu.com>
> Date: Sun, 09 Nov 2014 22:35:12 -0500
>
>> wouldnt this just break an existing ABI? You may need to introduce a
>> new attribute.
>
> He isn't breaking anything Jamal, he's just changing the internal
> macro name we use for the attribute's maximum length.
It is a _user space visible rename_, how about:
#define MAX_PHYS_ITEM_ID_LEN 32
#define MAX_PHYS_PORT_ID_LEN MAX_PHYS_ITEM_ID_LEN
I did miss the fact that the size didnt change.
cheers,
jamal
^ permalink raw reply
* [PATCH 4/4] qlcnic: remove pci_assigned_vfs() check while disabling VFs
From: Sathya Perla @ 2014-11-10 11:53 UTC (permalink / raw)
To: linux-pci, netdev; +Cc: ariel.elior, linux.nics, shahed.shaikh, ddutile
In-Reply-To: <1415620410-4937-1-git-send-email-sathya.perla@emulex.com>
From: Vasundhara Volam <vasundhara.volam@emulex.com>
The pci_assigned_vfs() check (while disabling VFs) is being moved to the
pci-sysfs.c file and will be done before invoking sriov_configure().
Signed-off-by: Vasundhara Volam <vasundhara.volam@emulex.com>
Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
---
.../net/ethernet/qlogic/qlcnic/qlcnic_sriov_pf.c | 10 ----------
1 files changed, 0 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sriov_pf.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sriov_pf.c
index a29538b..9802914 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sriov_pf.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sriov_pf.c
@@ -465,16 +465,6 @@ static int qlcnic_pci_sriov_disable(struct qlcnic_adapter *adapter)
{
struct net_device *netdev = adapter->netdev;
- if (pci_vfs_assigned(adapter->pdev)) {
- netdev_err(adapter->netdev,
- "SR-IOV VFs belonging to port %d are assigned to VMs. SR-IOV can not be disabled on this port\n",
- adapter->portnum);
- netdev_info(adapter->netdev,
- "Please detach SR-IOV VFs belonging to port %d from VMs, and then try to disable SR-IOV on this port\n",
- adapter->portnum);
- return -EPERM;
- }
-
qlcnic_sriov_pf_disable(adapter);
rtnl_lock();
--
1.7.1
^ permalink raw reply related
* [PATCH 3/4] i40e: remove pci_assigned_vfs() check while disabling VFs
From: Sathya Perla @ 2014-11-10 11:53 UTC (permalink / raw)
To: linux-pci, netdev; +Cc: ariel.elior, linux.nics, shahed.shaikh, ddutile
In-Reply-To: <1415620410-4937-1-git-send-email-sathya.perla@emulex.com>
From: Vasundhara Volam <vasundhara.volam@emulex.com>
The pci_assigned_vfs() check (while disabling VFs) is being moved to the
pci-sysfs.c file and will be done before invoking sriov_configure().
Signed-off-by: Vasundhara Volam <vasundhara.volam@emulex.com>
Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
---
drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 7 +------
1 files changed, 1 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
index fff3c27..0028a9a 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -951,12 +951,7 @@ int i40e_pci_sriov_configure(struct pci_dev *pdev, int num_vfs)
if (num_vfs)
return i40e_pci_sriov_enable(pdev, num_vfs);
- if (!pci_vfs_assigned(pf->pdev)) {
- i40e_free_vfs(pf);
- } else {
- dev_warn(&pdev->dev, "Unable to free VFs because some are assigned to VMs.\n");
- return -EINVAL;
- }
+ i40e_free_vfs(pf);
return 0;
}
--
1.7.1
^ permalink raw reply related
* [PATCH 2/4] bnx2x: remove pci_assigned_vfs() check while disabling VFs
From: Sathya Perla @ 2014-11-10 11:53 UTC (permalink / raw)
To: linux-pci, netdev; +Cc: ariel.elior, linux.nics, shahed.shaikh, ddutile
In-Reply-To: <1415620410-4937-1-git-send-email-sathya.perla@emulex.com>
From: Vasundhara Volam <vasundhara.volam@emulex.com>
The pci_assigned_vfs() check (while disabling VFs) is being moved to the
pci-sysfs.c file and will be done before invoking sriov_configure().
Signed-off-by: Vasundhara Volam <vasundhara.volam@emulex.com>
Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
---
drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c
index c88b20a..e90a10b 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c
@@ -2481,7 +2481,7 @@ int bnx2x_sriov_configure(struct pci_dev *dev, int num_vfs_param)
bp->requested_nr_virtfn = num_vfs_param;
if (num_vfs_param == 0) {
bnx2x_set_pf_tx_switching(bp, false);
- bnx2x_disable_sriov(bp);
+ pci_disable_sriov(bp->pdev);
return 0;
} else {
return bnx2x_enable_sriov(bp);
--
1.7.1
^ permalink raw reply related
* [PATCH 0/4] move pci_assivned_vfs() check (while disabling VFs) to pci sub-system
From: Sathya Perla @ 2014-11-10 11:53 UTC (permalink / raw)
To: linux-pci, netdev; +Cc: ariel.elior, linux.nics, shahed.shaikh, ddutile
A user must not be allowed to disable VFs while they are already assigned to
a guest. This check is being made in each individual driver that implements
the sriov_configure PCI method.
This patch-set fixes this code duplication by moving this check from
drivers to the sriov_nuvfs_store() routine just before invoking
sriov_configure() when num_vfs is equal to 0.
Vasundhara Volam (4):
pci: move pci_assivned_vfs() check while disabling VFs to pci
sub-system
bnx2x: remove pci_assigned_vfs() check while disabling VFs
i40e: remove pci_assigned_vfs() check while disabling VFs
qlcnic: remove pci_assigned_vfs() check while disabling VFs
drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c | 2 +-
drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 7 +------
.../net/ethernet/qlogic/qlcnic/qlcnic_sriov_pf.c | 10 ----------
drivers/pci/pci-sysfs.c | 5 +++++
4 files changed, 7 insertions(+), 17 deletions(-)
^ permalink raw reply
* [PATCH 1/4] pci: move pci_assivned_vfs() check while disabling VFs to pci sub-system
From: Sathya Perla @ 2014-11-10 11:53 UTC (permalink / raw)
To: linux-pci, netdev; +Cc: ariel.elior, linux.nics, shahed.shaikh, ddutile
In-Reply-To: <1415620410-4937-1-git-send-email-sathya.perla@emulex.com>
From: Vasundhara Volam <vasundhara.volam@emulex.com>
A user must not be allowed to disable VFs while they are already assigned to
a guest. This check is being made in each individual driver that implements
the sriov_configure PCI method.
This patch fixes this code duplication by moving this check to the
sriov_nuvfs_store() routine just before invoking sriov_configure() when
num_vfs is equal to 0.
Signed-off-by: Vasundhara Volam <vasundhara.volam@emulex.com>
Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
---
drivers/pci/pci-sysfs.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 2c6643f..6e65b47 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -477,6 +477,11 @@ static ssize_t sriov_numvfs_store(struct device *dev,
}
if (num_vfs == 0) {
+ if (pci_vfs_assigned(pdev)) {
+ dev_warn(&pdev->dev, "Cannot disable VFs while they are assigned\n");
+ return -EBUSY;
+ }
+
/* disable VFs */
ret = pdev->driver->sriov_configure(pdev, 0);
if (ret < 0)
--
1.7.1
^ 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