* Re: [patch iproute2 0/6] iproute2: add changes for switchdev
From: Roopa Prabhu @ 2014-12-04 14:26 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, jhs, 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, hemal
In-Reply-To: <1417683438-10935-1-git-send-email-jiri@resnulli.us>
On 12/4/14, 12:57 AM, Jiri Pirko wrote:
> Jiri Pirko (1):
> iproute2: ipa: show switch id
>
> Scott Feldman (5):
> bridge/fdb: fix statistics output spacing
> bridge/fdb: add flag/indication for FDB entry synced from offload
> device
> bridge/link: add new offload hwmode swdev
Ack to most patches but nack on this one. The todo list still has a note
to revist the flag to indicate switchdev offloads.
Exposing this to userspace does not help that.
> link: add missing IFLA_BRPORT_PROXYARP
> bridge/link: add learning_sync policy flag
>
> bridge/fdb.c | 4 +++-
> bridge/link.c | 17 +++++++++++++++--
> include/linux/if_bridge.h | 1 +
> include/linux/if_link.h | 3 +++
> include/linux/neighbour.h | 1 +
> ip/ipaddress.c | 8 ++++++++
> man/man8/bridge.8 | 19 ++++++++++++++-----
> 7 files changed, 45 insertions(+), 8 deletions(-)
>
^ permalink raw reply
* Re: [patch iproute2 1/6] iproute2: ipa: show switch id
From: Roopa Prabhu @ 2014-12-04 14:29 UTC (permalink / raw)
To: Andy Gospodarek
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, jhs,
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, bcrl, hemal
In-Reply-To: <20141204142015.GR27416@gospo.rtplab.test>
On 12/4/14, 6:20 AM, Andy Gospodarek wrote:
> On Thu, Dec 04, 2014 at 09:57:13AM +0100, Jiri Pirko wrote:
>> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>> ---
>> include/linux/if_link.h | 1 +
>> ip/ipaddress.c | 8 ++++++++
>> 2 files changed, 9 insertions(+)
>>
>> diff --git a/include/linux/if_link.h b/include/linux/if_link.h
>> index 4732063..a6e2594 100644
>> --- a/include/linux/if_link.h
>> +++ b/include/linux/if_link.h
>> @@ -145,6 +145,7 @@ enum {
>> IFLA_CARRIER,
>> IFLA_PHYS_PORT_ID,
>> IFLA_CARRIER_CHANGES,
>> + IFLA_PHYS_SWITCH_ID,
> Serious question for Stephen et al, once we take this to iproute2 are we
> going to be able to change the name but the string diplayed if needed?
>
> I had a patch that called this IFLA_PARENT_ID that I was using with the
> older github tree used by Jiri and Scott before these were in net-next.
> I wanted to submit that as a change to what became
> 82f2841291cfaf4d225aa1766424280254d3e3b2, but was waiting for things to
> be accepted and the dust settled.
>
> I like the parent/child/sibling nomenclature better for 4 reasons:
>
> - Most did not seem to like the term 'offload' since that term would be
> confusing with, GRO, GSO, etc.
> - A *significant* use case for many of the high-end ASICs in datacenters
> is routing.
> - switchid does not make sense in the OVS/flow case because it is all
> about flows, not switches or routers, and parent made sense there.
> - I wanted to combine this for use with SR-IOV use case, so one can more
> easily map PF->VF using this.
ack.., I have voted for calling it a generic parent id in the past.
>
> Can you give me a bit (a day) to clean-up that patch and submit an
> alternative proposal to these?
>
>> __IFLA_MAX
>> };
>>
>> diff --git a/ip/ipaddress.c b/ip/ipaddress.c
>> index 4d99324..bd36a07 100644
>> --- a/ip/ipaddress.c
>> +++ b/ip/ipaddress.c
>> @@ -589,6 +589,14 @@ int print_linkinfo(const struct sockaddr_nl *who,
>> b1, sizeof(b1)));
>> }
>>
>> + if (tb[IFLA_PHYS_SWITCH_ID]) {
>> + SPRINT_BUF(b1);
>> + fprintf(fp, "switchid %s ",
>> + hexstring_n2a(RTA_DATA(tb[IFLA_PHYS_SWITCH_ID]),
>> + RTA_PAYLOAD(tb[IFLA_PHYS_SWITCH_ID]),
>> + b1, sizeof(b1)));
>> + }
>> +
>> if (tb[IFLA_OPERSTATE])
>> print_operstate(fp, rta_getattr_u8(tb[IFLA_OPERSTATE]));
>>
>> --
>> 1.9.3
>>
^ permalink raw reply
* Re: [patch iproute2 0/6] iproute2: add changes for switchdev
From: Jamal Hadi Salim @ 2014-12-04 14:31 UTC (permalink / raw)
To: Roopa Prabhu, Andy Gospodarek
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, bcrl, hemal
In-Reply-To: <54806E1B.5010402@cumulusnetworks.com>
On 12/04/14 09:22, Roopa Prabhu wrote:
> On 12/4/14, 5:56 AM, Andy Gospodarek wrote:
> I think jamal meant the people in the cc (not netdev).
> I actually like the cc, but that probably cant last long because it is a
> lot of people.
There are 34 people on that list. I support copying all stake holders
but it is too large.
cheers,
jamal
^ permalink raw reply
* Re: [patch iproute2 1/6] iproute2: ipa: show switch id
From: Jiri Pirko @ 2014-12-04 14:33 UTC (permalink / raw)
To: Andy Gospodarek
Cc: netdev, davem, nhorman, andy, tgraf, 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, bcrl, hemal
In-Reply-To: <20141204142015.GR27416@gospo.rtplab.test>
Thu, Dec 04, 2014 at 03:20:15PM CET, gospo@cumulusnetworks.com wrote:
>On Thu, Dec 04, 2014 at 09:57:13AM +0100, Jiri Pirko wrote:
>> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>> ---
>> include/linux/if_link.h | 1 +
>> ip/ipaddress.c | 8 ++++++++
>> 2 files changed, 9 insertions(+)
>>
>> diff --git a/include/linux/if_link.h b/include/linux/if_link.h
>> index 4732063..a6e2594 100644
>> --- a/include/linux/if_link.h
>> +++ b/include/linux/if_link.h
>> @@ -145,6 +145,7 @@ enum {
>> IFLA_CARRIER,
>> IFLA_PHYS_PORT_ID,
>> IFLA_CARRIER_CHANGES,
>> + IFLA_PHYS_SWITCH_ID,
>
>Serious question for Stephen et al, once we take this to iproute2 are we
>going to be able to change the name but the string diplayed if needed?
>
>I had a patch that called this IFLA_PARENT_ID that I was using with the
>older github tree used by Jiri and Scott before these were in net-next.
>I wanted to submit that as a change to what became
>82f2841291cfaf4d225aa1766424280254d3e3b2, but was waiting for things to
>be accepted and the dust settled.
>
>I like the parent/child/sibling nomenclature better for 4 reasons:
>
>- Most did not seem to like the term 'offload' since that term would be
> confusing with, GRO, GSO, etc.
>- A *significant* use case for many of the high-end ASICs in datacenters
> is routing.
>- switchid does not make sense in the OVS/flow case because it is all
> about flows, not switches or routers, and parent made sense there.
well ovs is all about flows and has the "switch" word in the name. I
believe that people are talking about "switches" in case of these "flow
devices" as well. I see nothing wrong in that. I think that "switch"
became generic name for "packet forwarding machines".
"parent" is very generic and may mean 100 things...
>- I wanted to combine this for use with SR-IOV use case, so one can more
> easily map PF->VF using this.
Ugh, please don't mix this up with pf, vf. That is completely different
thing.
pf vf mapping is done in sysfs. In netlink, physportid is used for that
purpose. We can expose this phys port id for pf as well (as a different
attr) and we are done.
>
>Can you give me a bit (a day) to clean-up that patch and submit an
>alternative proposal to these?
>
>> __IFLA_MAX
>> };
>>
>> diff --git a/ip/ipaddress.c b/ip/ipaddress.c
>> index 4d99324..bd36a07 100644
>> --- a/ip/ipaddress.c
>> +++ b/ip/ipaddress.c
>> @@ -589,6 +589,14 @@ int print_linkinfo(const struct sockaddr_nl *who,
>> b1, sizeof(b1)));
>> }
>>
>> + if (tb[IFLA_PHYS_SWITCH_ID]) {
>> + SPRINT_BUF(b1);
>> + fprintf(fp, "switchid %s ",
>> + hexstring_n2a(RTA_DATA(tb[IFLA_PHYS_SWITCH_ID]),
>> + RTA_PAYLOAD(tb[IFLA_PHYS_SWITCH_ID]),
>> + b1, sizeof(b1)));
>> + }
>> +
>> if (tb[IFLA_OPERSTATE])
>> print_operstate(fp, rta_getattr_u8(tb[IFLA_OPERSTATE]));
>>
>> --
>> 1.9.3
>>
^ permalink raw reply
* Re: [patch iproute2 0/6] iproute2: add changes for switchdev
From: Jiri Pirko @ 2014-12-04 14:34 UTC (permalink / raw)
To: Roopa Prabhu
Cc: netdev, davem, nhorman, andy, tgraf, dborkman, ogerlitz, jesse,
pshelar, azhou, ben, stephen, jeffrey.t.kirsher, vyasevic,
xiyou.wangcong, john.r.fastabend, edumazet, jhs, 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, hemal
In-Reply-To: <54806F2A.2050109@cumulusnetworks.com>
Thu, Dec 04, 2014 at 03:26:50PM CET, roopa@cumulusnetworks.com wrote:
>On 12/4/14, 12:57 AM, Jiri Pirko wrote:
>>Jiri Pirko (1):
>> iproute2: ipa: show switch id
>>
>>Scott Feldman (5):
>> bridge/fdb: fix statistics output spacing
>> bridge/fdb: add flag/indication for FDB entry synced from offload
>> device
>> bridge/link: add new offload hwmode swdev
>Ack to most patches but nack on this one. The todo list still has a note to
>revist the flag to indicate switchdev offloads.
>Exposing this to userspace does not help that.
Hmm, note that this is already exposed to userspace, this patchset is
for iproute2 (userspace tool).
>
>> link: add missing IFLA_BRPORT_PROXYARP
>> bridge/link: add learning_sync policy flag
>>
>> bridge/fdb.c | 4 +++-
>> bridge/link.c | 17 +++++++++++++++--
>> include/linux/if_bridge.h | 1 +
>> include/linux/if_link.h | 3 +++
>> include/linux/neighbour.h | 1 +
>> ip/ipaddress.c | 8 ++++++++
>> man/man8/bridge.8 | 19 ++++++++++++++-----
>> 7 files changed, 45 insertions(+), 8 deletions(-)
>>
>
^ permalink raw reply
* Re: [patch iproute2 0/6] iproute2: add changes for switchdev
From: Roopa Prabhu @ 2014-12-04 14:45 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, jhs, 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, hemal
In-Reply-To: <20141204143400.GC1861@nanopsycho.orion>
On 12/4/14, 6:34 AM, Jiri Pirko wrote:
> Thu, Dec 04, 2014 at 03:26:50PM CET, roopa@cumulusnetworks.com wrote:
>> On 12/4/14, 12:57 AM, Jiri Pirko wrote:
>>> Jiri Pirko (1):
>>> iproute2: ipa: show switch id
>>>
>>> Scott Feldman (5):
>>> bridge/fdb: fix statistics output spacing
>>> bridge/fdb: add flag/indication for FDB entry synced from offload
>>> device
>>> bridge/link: add new offload hwmode swdev
>> Ack to most patches but nack on this one. The todo list still has a note to
>> revist the flag to indicate switchdev offloads.
>> Exposing this to userspace does not help that.
> Hmm, note that this is already exposed to userspace, this patchset is
> for iproute2 (userspace tool).
hmmm, all feedback on the switchdev patches seemed to indicate we can
change this later.
I don't see swdev mode being used in the kernel anywhere today.
I will send a patch to remove it. Its still in net-next and so can be
changed ?.
I was going to resend my patch to introduce a common offload flag for
all link objects.
It would be nice if all of them had a consistent flag to indicate hw
offload and iproute2 could display the same flag for all.
Including bonds and vxlan's.
>
>>> link: add missing IFLA_BRPORT_PROXYARP
>>> bridge/link: add learning_sync policy flag
>>>
>>> bridge/fdb.c | 4 +++-
>>> bridge/link.c | 17 +++++++++++++++--
>>> include/linux/if_bridge.h | 1 +
>>> include/linux/if_link.h | 3 +++
>>> include/linux/neighbour.h | 1 +
>>> ip/ipaddress.c | 8 ++++++++
>>> man/man8/bridge.8 | 19 ++++++++++++++-----
>>> 7 files changed, 45 insertions(+), 8 deletions(-)
>>>
^ permalink raw reply
* Re: [PATCH v2] drivers: net : cpsw: Update Kconfig for CPSW
From: Felipe Balbi @ 2014-12-04 14:48 UTC (permalink / raw)
To: Lokesh Vutla
Cc: netdev, davem, mugunthanvnm, linux-omap, grygorii.strashko, balbi,
nsekhar, t-kristo
In-Reply-To: <1417668869-25616-1-git-send-email-lokeshvutla@ti.com>
[-- Attachment #1: Type: text/plain, Size: 2035 bytes --]
On Thu, Dec 04, 2014 at 10:24:29AM +0530, Lokesh Vutla wrote:
> CPSW is present in AM33xx, AM43xx, DRA7xx.
> Updating the Kconfig to depend on ARCH_OMAP2PLUS instead of listing
> all SoC's.
>
> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
Reviewed-by: Felipe Balbi <balbi@ti.com>
> ---
> drivers/net/ethernet/ti/Kconfig | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/ti/Kconfig b/drivers/net/ethernet/ti/Kconfig
> index 5d8cb79..7051255 100644
> --- a/drivers/net/ethernet/ti/Kconfig
> +++ b/drivers/net/ethernet/ti/Kconfig
> @@ -5,7 +5,7 @@
> config NET_VENDOR_TI
> bool "Texas Instruments (TI) devices"
> default y
> - depends on PCI || EISA || AR7 || (ARM && (ARCH_DAVINCI || ARCH_OMAP3 || SOC_AM33XX || ARCH_KEYSTONE))
> + depends on PCI || EISA || AR7 || ARCH_DAVINCI || ARCH_OMAP2PLUS || ARCH_KEYSTONE
> ---help---
> If you have a network (Ethernet) card belonging to this class, say Y
> and read the Ethernet-HOWTO, available from
> @@ -32,7 +32,7 @@ config TI_DAVINCI_EMAC
>
> config TI_DAVINCI_MDIO
> tristate "TI DaVinci MDIO Support"
> - depends on ARM && ( ARCH_DAVINCI || ARCH_OMAP3 || SOC_AM33XX || ARCH_KEYSTONE )
> + depends on ARCH_DAVINCI || ARCH_OMAP2PLUS || ARCH_KEYSTONE
> select PHYLIB
> ---help---
> This driver supports TI's DaVinci MDIO module.
> @@ -42,7 +42,7 @@ config TI_DAVINCI_MDIO
>
> config TI_DAVINCI_CPDMA
> tristate "TI DaVinci CPDMA Support"
> - depends on ARM && ( ARCH_DAVINCI || ARCH_OMAP3 || SOC_AM33XX )
> + depends on ARCH_DAVINCI || ARCH_OMAP2PLUS
> ---help---
> This driver supports TI's DaVinci CPDMA dma engine.
>
> @@ -58,7 +58,7 @@ config TI_CPSW_PHY_SEL
>
> config TI_CPSW
> tristate "TI CPSW Switch Support"
> - depends on ARM && (ARCH_DAVINCI || SOC_AM33XX)
> + depends on ARCH_DAVINCI || ARCH_OMAP2PLUS
> select TI_DAVINCI_CPDMA
> select TI_DAVINCI_MDIO
> select TI_CPSW_PHY_SEL
> --
> 1.9.1
>
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [patch iproute2 1/6] iproute2: ipa: show switch id
From: Andy Gospodarek @ 2014-12-04 14:57 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, 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, bcrl, hemal
In-Reply-To: <20141204143306.GB1861@nanopsycho.orion>
On Thu, Dec 04, 2014 at 03:33:06PM +0100, Jiri Pirko wrote:
> Thu, Dec 04, 2014 at 03:20:15PM CET, gospo@cumulusnetworks.com wrote:
> >On Thu, Dec 04, 2014 at 09:57:13AM +0100, Jiri Pirko wrote:
> >> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
> >> ---
> >> include/linux/if_link.h | 1 +
> >> ip/ipaddress.c | 8 ++++++++
> >> 2 files changed, 9 insertions(+)
> >>
> >> diff --git a/include/linux/if_link.h b/include/linux/if_link.h
> >> index 4732063..a6e2594 100644
> >> --- a/include/linux/if_link.h
> >> +++ b/include/linux/if_link.h
> >> @@ -145,6 +145,7 @@ enum {
> >> IFLA_CARRIER,
> >> IFLA_PHYS_PORT_ID,
> >> IFLA_CARRIER_CHANGES,
> >> + IFLA_PHYS_SWITCH_ID,
> >
> >Serious question for Stephen et al, once we take this to iproute2 are we
> >going to be able to change the name but the string diplayed if needed?
> >
> >I had a patch that called this IFLA_PARENT_ID that I was using with the
> >older github tree used by Jiri and Scott before these were in net-next.
> >I wanted to submit that as a change to what became
> >82f2841291cfaf4d225aa1766424280254d3e3b2, but was waiting for things to
> >be accepted and the dust settled.
> >
> >I like the parent/child/sibling nomenclature better for 4 reasons:
> >
> >- Most did not seem to like the term 'offload' since that term would be
> > confusing with, GRO, GSO, etc.
> >- A *significant* use case for many of the high-end ASICs in datacenters
> > is routing.
> >- switchid does not make sense in the OVS/flow case because it is all
> > about flows, not switches or routers, and parent made sense there.
>
> well ovs is all about flows and has the "switch" word in the name. I
> believe that people are talking about "switches" in case of these "flow
> devices" as well. I see nothing wrong in that. I think that "switch"
> became generic name for "packet forwarding machines".
Just because one chose to use it that way does not mean I agree with it
and that we should copy their bad decision. :-)
> "parent" is very generic and may mean 100 things...
But in this case, it means 1 thing. The netdev you are using is
connected to another device that controls it rather than being just a
NIC.
>
>
> >- I wanted to combine this for use with SR-IOV use case, so one can more
> > easily map PF->VF using this.
>
> Ugh, please don't mix this up with pf, vf. That is completely different
> thing.
>
> pf vf mapping is done in sysfs. In netlink, physportid is used for that
> purpose. We can expose this phys port id for pf as well (as a different
> attr) and we are done.
I know that attribute is there, but I find it more valuable for
solutions like nPAR than for PF/VF use-case. Parent/child relationship
makes more sense to me since for forwarding will be controlled by the
embedded switch on those devices. (Notice I specifically chose not to
use 'master' since that is already overloaded by bridge, bonding,
teaming, etc.)
>
> >
> >Can you give me a bit (a day) to clean-up that patch and submit an
> >alternative proposal to these?
> >
> >> __IFLA_MAX
> >> };
> >>
> >> diff --git a/ip/ipaddress.c b/ip/ipaddress.c
> >> index 4d99324..bd36a07 100644
> >> --- a/ip/ipaddress.c
> >> +++ b/ip/ipaddress.c
> >> @@ -589,6 +589,14 @@ int print_linkinfo(const struct sockaddr_nl *who,
> >> b1, sizeof(b1)));
> >> }
> >>
> >> + if (tb[IFLA_PHYS_SWITCH_ID]) {
> >> + SPRINT_BUF(b1);
> >> + fprintf(fp, "switchid %s ",
> >> + hexstring_n2a(RTA_DATA(tb[IFLA_PHYS_SWITCH_ID]),
> >> + RTA_PAYLOAD(tb[IFLA_PHYS_SWITCH_ID]),
> >> + b1, sizeof(b1)));
> >> + }
> >> +
> >> if (tb[IFLA_OPERSTATE])
> >> print_operstate(fp, rta_getattr_u8(tb[IFLA_OPERSTATE]));
> >>
> >> --
> >> 1.9.3
> >>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [patch iproute2 1/6] iproute2: ipa: show switch id
From: Jiri Pirko @ 2014-12-04 15:12 UTC (permalink / raw)
To: Andy Gospodarek
Cc: netdev, davem, nhorman, andy, tgraf, 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, bcrl, hemal
In-Reply-To: <20141204145743.GS27416@gospo.rtplab.test>
Thu, Dec 04, 2014 at 03:57:43PM CET, gospo@cumulusnetworks.com wrote:
>On Thu, Dec 04, 2014 at 03:33:06PM +0100, Jiri Pirko wrote:
>> Thu, Dec 04, 2014 at 03:20:15PM CET, gospo@cumulusnetworks.com wrote:
>> >On Thu, Dec 04, 2014 at 09:57:13AM +0100, Jiri Pirko wrote:
>> >> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>> >> ---
>> >> include/linux/if_link.h | 1 +
>> >> ip/ipaddress.c | 8 ++++++++
>> >> 2 files changed, 9 insertions(+)
>> >>
>> >> diff --git a/include/linux/if_link.h b/include/linux/if_link.h
>> >> index 4732063..a6e2594 100644
>> >> --- a/include/linux/if_link.h
>> >> +++ b/include/linux/if_link.h
>> >> @@ -145,6 +145,7 @@ enum {
>> >> IFLA_CARRIER,
>> >> IFLA_PHYS_PORT_ID,
>> >> IFLA_CARRIER_CHANGES,
>> >> + IFLA_PHYS_SWITCH_ID,
>> >
>> >Serious question for Stephen et al, once we take this to iproute2 are we
>> >going to be able to change the name but the string diplayed if needed?
>> >
>> >I had a patch that called this IFLA_PARENT_ID that I was using with the
>> >older github tree used by Jiri and Scott before these were in net-next.
>> >I wanted to submit that as a change to what became
>> >82f2841291cfaf4d225aa1766424280254d3e3b2, but was waiting for things to
>> >be accepted and the dust settled.
>> >
>> >I like the parent/child/sibling nomenclature better for 4 reasons:
>> >
>> >- Most did not seem to like the term 'offload' since that term would be
>> > confusing with, GRO, GSO, etc.
>> >- A *significant* use case for many of the high-end ASICs in datacenters
>> > is routing.
>> >- switchid does not make sense in the OVS/flow case because it is all
>> > about flows, not switches or routers, and parent made sense there.
>>
>> well ovs is all about flows and has the "switch" word in the name. I
>> believe that people are talking about "switches" in case of these "flow
>> devices" as well. I see nothing wrong in that. I think that "switch"
>> became generic name for "packet forwarding machines".
>
>Just because one chose to use it that way does not mean I agree with it
>and that we should copy their bad decision. :-)
>
>> "parent" is very generic and may mean 100 things...
>
>But in this case, it means 1 thing. The netdev you are using is
>connected to another device that controls it rather than being just a
>NIC.
when we discussed linkage now called upper/lower devices, there was a
proposal to use "parent" as well. And then it ment only 1 thing as well.
Why for example pf is not parent of vf? Using "parent" word here and
assume that it just makes sense for this case is IMHO wrong. Too
generic. "switch_parent" - that I am okay with.
>
>>
>>
>> >- I wanted to combine this for use with SR-IOV use case, so one can more
>> > easily map PF->VF using this.
>>
>> Ugh, please don't mix this up with pf, vf. That is completely different
>> thing.
>>
>> pf vf mapping is done in sysfs. In netlink, physportid is used for that
>> purpose. We can expose this phys port id for pf as well (as a different
>> attr) and we are done.
>
>I know that attribute is there, but I find it more valuable for
>solutions like nPAR than for PF/VF use-case.
Why it is not good enough for pf/vf? There are SR-IOV drivers using
this:
$ git grep ndo_get_phys_port_id
drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c: .ndo_get_phys_port_id = bnx2x_get_phys_port_id,
drivers/net/ethernet/intel/i40e/i40e_main.c: .ndo_get_phys_port_id = i40e_get_phys_port_id,
drivers/net/ethernet/mellanox/mlx4/en_netdev.c: .ndo_get_phys_port_id = mlx4_en_get_phys_port_id,
drivers/net/ethernet/mellanox/mlx4/en_netdev.c: .ndo_get_phys_port_id = mlx4_en_get_phys_port_id,
drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c: .ndo_get_phys_port_id = qlcnic_get_phys_port_id,
>Parent/child relationship
>makes more sense to me since for forwarding will be controlled by the
>embedded switch on those devices. (Notice I specifically chose not to
>use 'master' since that is already overloaded by bridge, bonding,
>teaming, etc.)
But is that embedded switch the same thing as PF? I doubt that. Let the
aspect of PF/VF aside and look at it as on ordinary switch. The eswitch
driver needs te be implemented and add netdevs for internal ports (they
do not exist now).
You have to look at PF/VF and eSwitch as on separate things.
>
>>
>> >
>> >Can you give me a bit (a day) to clean-up that patch and submit an
>> >alternative proposal to these?
>> >
>> >> __IFLA_MAX
>> >> };
>> >>
>> >> diff --git a/ip/ipaddress.c b/ip/ipaddress.c
>> >> index 4d99324..bd36a07 100644
>> >> --- a/ip/ipaddress.c
>> >> +++ b/ip/ipaddress.c
>> >> @@ -589,6 +589,14 @@ int print_linkinfo(const struct sockaddr_nl *who,
>> >> b1, sizeof(b1)));
>> >> }
>> >>
>> >> + if (tb[IFLA_PHYS_SWITCH_ID]) {
>> >> + SPRINT_BUF(b1);
>> >> + fprintf(fp, "switchid %s ",
>> >> + hexstring_n2a(RTA_DATA(tb[IFLA_PHYS_SWITCH_ID]),
>> >> + RTA_PAYLOAD(tb[IFLA_PHYS_SWITCH_ID]),
>> >> + b1, sizeof(b1)));
>> >> + }
>> >> +
>> >> if (tb[IFLA_OPERSTATE])
>> >> print_operstate(fp, rta_getattr_u8(tb[IFLA_OPERSTATE]));
>> >>
>> >> --
>> >> 1.9.3
>> >>
>> --
>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>--
>To unsubscribe from this list: send the line "unsubscribe netdev" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [patch iproute2 1/6] iproute2: ipa: show switch id
From: Jiri Pirko @ 2014-12-04 15:15 UTC (permalink / raw)
To: Andy Gospodarek
Cc: netdev, davem, nhorman, andy, tgraf, 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, bcrl, hemal
In-Reply-To: <20141204145743.GS27416@gospo.rtplab.test>
Thu, Dec 04, 2014 at 03:57:43PM CET, gospo@cumulusnetworks.com wrote:
>On Thu, Dec 04, 2014 at 03:33:06PM +0100, Jiri Pirko wrote:
>> Thu, Dec 04, 2014 at 03:20:15PM CET, gospo@cumulusnetworks.com wrote:
>> >On Thu, Dec 04, 2014 at 09:57:13AM +0100, Jiri Pirko wrote:
>> >> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>> >> ---
>> >> include/linux/if_link.h | 1 +
>> >> ip/ipaddress.c | 8 ++++++++
>> >> 2 files changed, 9 insertions(+)
>> >>
>> >> diff --git a/include/linux/if_link.h b/include/linux/if_link.h
>> >> index 4732063..a6e2594 100644
>> >> --- a/include/linux/if_link.h
>> >> +++ b/include/linux/if_link.h
>> >> @@ -145,6 +145,7 @@ enum {
>> >> IFLA_CARRIER,
>> >> IFLA_PHYS_PORT_ID,
>> >> IFLA_CARRIER_CHANGES,
>> >> + IFLA_PHYS_SWITCH_ID,
>> >
>> >Serious question for Stephen et al, once we take this to iproute2 are we
>> >going to be able to change the name but the string diplayed if needed?
>> >
>> >I had a patch that called this IFLA_PARENT_ID that I was using with the
>> >older github tree used by Jiri and Scott before these were in net-next.
>> >I wanted to submit that as a change to what became
>> >82f2841291cfaf4d225aa1766424280254d3e3b2, but was waiting for things to
>> >be accepted and the dust settled.
>> >
>> >I like the parent/child/sibling nomenclature better for 4 reasons:
>> >
>> >- Most did not seem to like the term 'offload' since that term would be
>> > confusing with, GRO, GSO, etc.
>> >- A *significant* use case for many of the high-end ASICs in datacenters
>> > is routing.
>> >- switchid does not make sense in the OVS/flow case because it is all
>> > about flows, not switches or routers, and parent made sense there.
>>
>> well ovs is all about flows and has the "switch" word in the name. I
>> believe that people are talking about "switches" in case of these "flow
>> devices" as well. I see nothing wrong in that. I think that "switch"
>> became generic name for "packet forwarding machines".
>
>Just because one chose to use it that way does not mean I agree with it
>and that we should copy their bad decision. :-)
>
>> "parent" is very generic and may mean 100 things...
>
>But in this case, it means 1 thing. The netdev you are using is
>connected to another device that controls it rather than being just a
>NIC.
>
>>
>>
>> >- I wanted to combine this for use with SR-IOV use case, so one can more
>> > easily map PF->VF using this.
>>
>> Ugh, please don't mix this up with pf, vf. That is completely different
>> thing.
>>
>> pf vf mapping is done in sysfs. In netlink, physportid is used for that
>> purpose. We can expose this phys port id for pf as well (as a different
>> attr) and we are done.
>
>I know that attribute is there, but I find it more valuable for
>solutions like nPAR than for PF/VF use-case. Parent/child relationship
>makes more sense to me since for forwarding will be controlled by the
>embedded switch on those devices. (Notice I specifically chose not to
see, "embedded switch", not "embedded parent" or "embedded offload". And
yes, the "embedded switch" also may (and some of them do today) work
with flows.
>use 'master' since that is already overloaded by bridge, bonding,
>teaming, etc.)
>
>>
>> >
>> >Can you give me a bit (a day) to clean-up that patch and submit an
>> >alternative proposal to these?
>> >
>> >> __IFLA_MAX
>> >> };
>> >>
>> >> diff --git a/ip/ipaddress.c b/ip/ipaddress.c
>> >> index 4d99324..bd36a07 100644
>> >> --- a/ip/ipaddress.c
>> >> +++ b/ip/ipaddress.c
>> >> @@ -589,6 +589,14 @@ int print_linkinfo(const struct sockaddr_nl *who,
>> >> b1, sizeof(b1)));
>> >> }
>> >>
>> >> + if (tb[IFLA_PHYS_SWITCH_ID]) {
>> >> + SPRINT_BUF(b1);
>> >> + fprintf(fp, "switchid %s ",
>> >> + hexstring_n2a(RTA_DATA(tb[IFLA_PHYS_SWITCH_ID]),
>> >> + RTA_PAYLOAD(tb[IFLA_PHYS_SWITCH_ID]),
>> >> + b1, sizeof(b1)));
>> >> + }
>> >> +
>> >> if (tb[IFLA_OPERSTATE])
>> >> print_operstate(fp, rta_getattr_u8(tb[IFLA_OPERSTATE]));
>> >>
>> >> --
>> >> 1.9.3
>> >>
>> --
>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>--
>To unsubscribe from this list: send the line "unsubscribe netdev" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: Where exactly will arch_fast_hash be used
From: Thomas Graf @ 2014-12-04 15:26 UTC (permalink / raw)
To: Herbert Xu
Cc: Daniel Borkmann, David S. Miller, Theodore Ts'o, netdev,
Linux Kernel Mailing List
In-Reply-To: <20141204081147.GA19030@gondor.apana.org.au>
On 12/04/14 at 04:11pm, Herbert Xu wrote:
> Hi:
>
> While working on rhashtable it came to me that this whole concept
> of arch_fast_hash is flawed. CRCs are linear functions so it's
> fairly easy for an attacker to identify collisions or at least
> eliminate a large amount of search space (e.g., controlling the
> last bit of the hash result is almost trivial, even when you add
> a random seed).
>
> So what exactly are we going to use arch_fast_hash for? Presumably
> it's places where security is never goint to be an issue, right?
>
> Even if security wasn't an issue, straight CRC32 has really poor
> lower-order bit distribution, which makes it a terrible choice for
> a hash table that simply uses the lower-order bits.
As Daniel pointed out, this work originated for the OVS edge use
case where security is of less concern and the rehashing is
sufficient. Identifying collisions is less of interest as the user
space fall back provides a greater surface for an attack.
^ permalink raw reply
* Re: [patch iproute2 1/6] iproute2: ipa: show switch id
From: Andy Gospodarek @ 2014-12-04 15:28 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, 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, bcrl, hemal
In-Reply-To: <20141204151512.GE1861@nanopsycho.orion>
On Thu, Dec 04, 2014 at 04:15:12PM +0100, Jiri Pirko wrote:
> Thu, Dec 04, 2014 at 03:57:43PM CET, gospo@cumulusnetworks.com wrote:
> >On Thu, Dec 04, 2014 at 03:33:06PM +0100, Jiri Pirko wrote:
> >> Thu, Dec 04, 2014 at 03:20:15PM CET, gospo@cumulusnetworks.com wrote:
> >> >On Thu, Dec 04, 2014 at 09:57:13AM +0100, Jiri Pirko wrote:
> >> >> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
> >> >> ---
> >> >> include/linux/if_link.h | 1 +
> >> >> ip/ipaddress.c | 8 ++++++++
> >> >> 2 files changed, 9 insertions(+)
> >> >>
> >> >> diff --git a/include/linux/if_link.h b/include/linux/if_link.h
> >> >> index 4732063..a6e2594 100644
> >> >> --- a/include/linux/if_link.h
> >> >> +++ b/include/linux/if_link.h
> >> >> @@ -145,6 +145,7 @@ enum {
> >> >> IFLA_CARRIER,
> >> >> IFLA_PHYS_PORT_ID,
> >> >> IFLA_CARRIER_CHANGES,
> >> >> + IFLA_PHYS_SWITCH_ID,
> >> >
> >> >Serious question for Stephen et al, once we take this to iproute2 are we
> >> >going to be able to change the name but the string diplayed if needed?
> >> >
> >> >I had a patch that called this IFLA_PARENT_ID that I was using with the
> >> >older github tree used by Jiri and Scott before these were in net-next.
> >> >I wanted to submit that as a change to what became
> >> >82f2841291cfaf4d225aa1766424280254d3e3b2, but was waiting for things to
> >> >be accepted and the dust settled.
> >> >
> >> >I like the parent/child/sibling nomenclature better for 4 reasons:
> >> >
> >> >- Most did not seem to like the term 'offload' since that term would be
> >> > confusing with, GRO, GSO, etc.
> >> >- A *significant* use case for many of the high-end ASICs in datacenters
> >> > is routing.
> >> >- switchid does not make sense in the OVS/flow case because it is all
> >> > about flows, not switches or routers, and parent made sense there.
> >>
> >> well ovs is all about flows and has the "switch" word in the name. I
> >> believe that people are talking about "switches" in case of these "flow
> >> devices" as well. I see nothing wrong in that. I think that "switch"
> >> became generic name for "packet forwarding machines".
> >
> >Just because one chose to use it that way does not mean I agree with it
> >and that we should copy their bad decision. :-)
> >
> >> "parent" is very generic and may mean 100 things...
> >
> >But in this case, it means 1 thing. The netdev you are using is
> >connected to another device that controls it rather than being just a
> >NIC.
> >
> >>
> >>
> >> >- I wanted to combine this for use with SR-IOV use case, so one can more
> >> > easily map PF->VF using this.
> >>
> >> Ugh, please don't mix this up with pf, vf. That is completely different
> >> thing.
> >>
> >> pf vf mapping is done in sysfs. In netlink, physportid is used for that
> >> purpose. We can expose this phys port id for pf as well (as a different
> >> attr) and we are done.
> >
> >I know that attribute is there, but I find it more valuable for
> >solutions like nPAR than for PF/VF use-case. Parent/child relationship
> >makes more sense to me since for forwarding will be controlled by the
> >embedded switch on those devices. (Notice I specifically chose not to
>
> see, "embedded switch", not "embedded parent" or "embedded offload". And
> yes, the "embedded switch" also may (and some of them do today) work
> with flows.
It's fine to exclude the PF/VF part for now since you seem disagree with
that as a valid reason. The other 3 reasons are in my mind, so that was
why I asked to Stephen would mind waiting for me to post my set. I'm
not asking for you to do anything here but to be patient and let me get
the set out.
>
>
> >use 'master' since that is already overloaded by bridge, bonding,
> >teaming, etc.)
> >
> >>
> >> >
> >> >Can you give me a bit (a day) to clean-up that patch and submit an
> >> >alternative proposal to these?
> >> >
> >> >> __IFLA_MAX
> >> >> };
> >> >>
> >> >> diff --git a/ip/ipaddress.c b/ip/ipaddress.c
> >> >> index 4d99324..bd36a07 100644
> >> >> --- a/ip/ipaddress.c
> >> >> +++ b/ip/ipaddress.c
> >> >> @@ -589,6 +589,14 @@ int print_linkinfo(const struct sockaddr_nl *who,
> >> >> b1, sizeof(b1)));
> >> >> }
> >> >>
> >> >> + if (tb[IFLA_PHYS_SWITCH_ID]) {
> >> >> + SPRINT_BUF(b1);
> >> >> + fprintf(fp, "switchid %s ",
> >> >> + hexstring_n2a(RTA_DATA(tb[IFLA_PHYS_SWITCH_ID]),
> >> >> + RTA_PAYLOAD(tb[IFLA_PHYS_SWITCH_ID]),
> >> >> + b1, sizeof(b1)));
> >> >> + }
> >> >> +
> >> >> if (tb[IFLA_OPERSTATE])
> >> >> print_operstate(fp, rta_getattr_u8(tb[IFLA_OPERSTATE]));
> >> >>
> >> >> --
> >> >> 1.9.3
> >> >>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe netdev" in
> >> the body of a message to majordomo@vger.kernel.org
> >> More majordomo info at http://vger.kernel.org/majordomo-info.html
> >--
> >To unsubscribe from this list: send the line "unsubscribe netdev" in
> >the body of a message to majordomo@vger.kernel.org
> >More majordomo info at http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: Where exactly will arch_fast_hash be used
From: Herbert Xu @ 2014-12-04 15:29 UTC (permalink / raw)
To: Thomas Graf
Cc: Daniel Borkmann, David S. Miller, Theodore Ts'o, netdev,
Linux Kernel Mailing List
In-Reply-To: <20141204152637.GA32140@casper.infradead.org>
On Thu, Dec 04, 2014 at 03:26:37PM +0000, Thomas Graf wrote:
>
> As Daniel pointed out, this work originated for the OVS edge use
> case where security is of less concern and the rehashing is
> sufficient. Identifying collisions is less of interest as the user
> space fall back provides a greater surface for an attack.
Well in that case the current setup I think is very misleading.
It's inviting unsuspecting kernel developers to use it as a hash
function for general hash tables, which AFAICS is something that
it fails at miserably.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [patch iproute2 1/6] iproute2: ipa: show switch id
From: Thomas Graf @ 2014-12-04 15:37 UTC (permalink / raw)
To: Andy Gospodarek
Cc: Jiri Pirko, 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, bcrl, hema
In-Reply-To: <20141204145743.GS27416@gospo.rtplab.test>
On 12/04/14 at 09:57am, Andy Gospodarek wrote:
> I know that attribute is there, but I find it more valuable for
> solutions like nPAR than for PF/VF use-case. Parent/child relationship
> makes more sense to me since for forwarding will be controlled by the
> embedded switch on those devices. (Notice I specifically chose not to
> use 'master' since that is already overloaded by bridge, bonding,
> teaming, etc.)
So that would make it link, master, and parent. Not sure we can make
it any more confusing ;-)
TBH, I'm really struggling on how to best name this. Trying to reach
for 100% correctness leads to generic, amgbigous names.
^ permalink raw reply
* Re: Where exactly will arch_fast_hash be used
From: Thomas Graf @ 2014-12-04 15:39 UTC (permalink / raw)
To: Herbert Xu
Cc: Daniel Borkmann, David S. Miller, Theodore Ts'o, netdev,
Linux Kernel Mailing List
In-Reply-To: <20141204152929.GA22075@gondor.apana.org.au>
On 12/04/14 at 11:29pm, Herbert Xu wrote:
> On Thu, Dec 04, 2014 at 03:26:37PM +0000, Thomas Graf wrote:
> >
> > As Daniel pointed out, this work originated for the OVS edge use
> > case where security is of less concern and the rehashing is
> > sufficient. Identifying collisions is less of interest as the user
> > space fall back provides a greater surface for an attack.
>
> Well in that case the current setup I think is very misleading.
> It's inviting unsuspecting kernel developers to use it as a hash
> function for general hash tables, which AFAICS is something that
> it fails at miserably.
Well, it's called fast hash and not secure hash ;-) but a clear hint
definitely wouldn't hurt.
I wasn't aware of the distribution weakness of the lower bits. Do
you have a reference to more information?
^ permalink raw reply
* Re: Where exactly will arch_fast_hash be used
From: Daniel Borkmann @ 2014-12-04 15:43 UTC (permalink / raw)
To: Thomas Graf
Cc: Herbert Xu, David S. Miller, Theodore Ts'o, netdev,
Linux Kernel Mailing List
In-Reply-To: <20141204153938.GC32140@casper.infradead.org>
On 12/04/2014 04:39 PM, Thomas Graf wrote:
> On 12/04/14 at 11:29pm, Herbert Xu wrote:
>> On Thu, Dec 04, 2014 at 03:26:37PM +0000, Thomas Graf wrote:
>>>
>>> As Daniel pointed out, this work originated for the OVS edge use
>>> case where security is of less concern and the rehashing is
>>> sufficient. Identifying collisions is less of interest as the user
>>> space fall back provides a greater surface for an attack.
>>
>> Well in that case the current setup I think is very misleading.
>> It's inviting unsuspecting kernel developers to use it as a hash
>> function for general hash tables, which AFAICS is something that
>> it fails at miserably.
>
> Well, it's called fast hash and not secure hash ;-) but a clear hint
> definitely wouldn't hurt.
Hm, I thought the kernel doc on arch_fast_hash() in include/linux/hash.h
would give enough of a hint ...
^ permalink raw reply
* Re: Where exactly will arch_fast_hash be used
From: Herbert Xu @ 2014-12-04 15:47 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Thomas Graf, David S. Miller, Theodore Ts'o, netdev,
Linux Kernel Mailing List
In-Reply-To: <54808123.8000704@redhat.com>
On Thu, Dec 04, 2014 at 04:43:31PM +0100, Daniel Borkmann wrote:
>
> Hm, I thought the kernel doc on arch_fast_hash() in include/linux/hash.h
> would give enough of a hint ...
I think something more explicit like "do not use this in a hash
table unless you know what you're doing" might be needed.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: Where exactly will arch_fast_hash be used
From: Daniel Borkmann @ 2014-12-04 15:51 UTC (permalink / raw)
To: Herbert Xu
Cc: Thomas Graf, David S. Miller, Theodore Ts'o, netdev,
Linux Kernel Mailing List
In-Reply-To: <20141204154708.GA22185@gondor.apana.org.au>
On 12/04/2014 04:47 PM, Herbert Xu wrote:
> On Thu, Dec 04, 2014 at 04:43:31PM +0100, Daniel Borkmann wrote:
>>
>> Hm, I thought the kernel doc on arch_fast_hash() in include/linux/hash.h
>> would give enough of a hint ...
>
> I think something more explicit like "do not use this in a hash
> table unless you know what you're doing" might be needed.
Ok, I'm fine with that, do you want to submit a patch on top of, or
after Hannes' improvements [1] got in? Thanks a lot, Herbert.
[1] http://patchwork.ozlabs.org/patch/417761/
^ permalink raw reply
* Re: [PATCH net-next] arch_fast_hash: avoid indirect function calls and implement hash in asm
From: Herbert Xu @ 2014-12-04 15:56 UTC (permalink / raw)
To: Hannes Frederic Sowa
Cc: netdev, Jay Vosburgh, Thomas Graf, Daniel Borkmann, Eric Dumazet,
Linux Kernel Mailing List
In-Reply-To: <e77237174e1b8d743ef5171e05abde82bc54af37.1417696901.git.hannes@stressinduktion.org>
On Thu, Dec 04, 2014 at 02:08:50PM +0100, Hannes Frederic Sowa wrote:
> By default the arch_fast_hash hashing function pointers are initialized
> to jhash(2). If during boot-up a CPU with SSE4.2 is detected they get
> updated to the CRC32 ones. This dispatching scheme incurs a function
> pointer lookup and indirect call for every hashing operation.
Just curious, is jhash actually faster than generic C CRC32 on
common platforms?
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* RE: Where exactly will arch_fast_hash be used
From: David Laight @ 2014-12-04 15:56 UTC (permalink / raw)
To: 'Herbert Xu', Daniel Borkmann
Cc: Thomas Graf, David S. Miller, Theodore Ts'o,
netdev@vger.kernel.org, Linux Kernel Mailing List
In-Reply-To: <20141204154708.GA22185@gondor.apana.org.au>
From: Herbert Xu
> On Thu, Dec 04, 2014 at 04:43:31PM +0100, Daniel Borkmann wrote:
> >
> > Hm, I thought the kernel doc on arch_fast_hash() in include/linux/hash.h
> > would give enough of a hint ...
>
> I think something more explicit like "do not use this in a hash
> table unless you know what you're doing" might be needed.
That isn't really helpful. Maybe something more like:
"This CRC algorithm used by this hash is 'linear', ie hash(a xor b) ==
hash(a) xor hash(b). This means that it is relatively easy for a remote
attacker to generate multiple items with the same hash."
David
^ permalink raw reply
* Re: [PATCH] x86: bpf_jit_comp: simplify trivial boolean return
From: Alexei Starovoitov @ 2014-12-04 15:56 UTC (permalink / raw)
To: Joe Perches
Cc: David Laight, Quentin Lambert, David S. Miller, Alexey Kuznetsov,
James Morris, Hideaki YOSHIFUJI, Patrick McHardy, Thomas Gleixner,
Ingo Molnar, H. Peter Anvin, x86@kernel.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <1417685187.2721.1.camel@perches.com>
On Thu, Dec 4, 2014 at 1:26 AM, Joe Perches <joe@perches.com> wrote:
> On Thu, 2014-11-27 at 10:49 -0800, Joe Perches wrote:
>> On Thu, 2014-11-27 at 12:25 +0000, David Laight wrote:
>> > Why the change in data?
>>
>> btw: without gcov and using -O2
>>
>> $ size arch/x86/net/bpf_jit_comp.o*
>> text data bss dec hex filename
>> 9671 4 0 9675 25cb arch/x86/net/bpf_jit_comp.o.new
>> 10679 4 0 10683 29bb arch/x86/net/bpf_jit_comp.o.old
>
> Alexei?
>
> Is this 10% reduction in size a good reason to change the code?
yes.
I believe you're seeing it with gcc 4.9. I wanted to double
check what 4.6 and 4.7 are doing. If they're not suddenly
increase code size then resubmit it for inclusion please.
^ permalink raw reply
* Re: Possible regression: "gre: Use inner mac length when computing tunnel length"
From: Tom Herbert @ 2014-12-04 16:00 UTC (permalink / raw)
To: Timo Teras; +Cc: Alexander Duyck, Linux Netdev List
In-Reply-To: <20141204141616.185b88c4@vostro>
A fix is pending for net. Please try if you can.
Tom
diff --git a/net/ipv4/gre_offload.c b/net/ipv4/gre_offload.c
index bb5947b..51973dd 100644
--- a/net/ipv4/gre_offload.c
+++ b/net/ipv4/gre_offload.c
@@ -247,6 +247,9 @@ static int gre_gro_complete(struct sk_buff *skb, int nhoff)
err = ptype->callbacks.gro_complete(skb, nhoff + grehlen);
rcu_read_unlock();
+
+ skb_set_inner_mac_header(skb, nhoff + grehlen);
+
return err;
}
On Thu, Dec 4, 2014 at 4:16 AM, Timo Teras <timo.teras@iki.fi> wrote:
> Hi,
>
> After upgrading to latest 3.14.24 or newer, I noticed a weird TSO bug
> in the "dmvpn" setup I use. And seems 3.14.23 works just fine. So the
> commit 14051f0452a2c26a "gre: Use inner mac length when computing
> tunnel length" would appear to be the related commit (but have not yet
> tested this).
>
> In practice what happens is that forwarding path between ethX (or vlanX)
> and gre1 gets broken.
>
> There's probably two differences to the "regular" gre tunnel case:
> - it's nbma mode, meaning the gre header is inserted via slightly
> different code path
> - the gre1 packets are IPsec encrypted in transport mode
>
> As additional detail, doing "ethtool -K gre1 tso off" will workaround
> the issue, so it is clearly tso issue pointing even further to the
> commit in question.
>
> Is this something the suspected patch could cause? Any suggestions
> what to test more?
>
> Thanks,
> Timo
>
^ permalink raw reply related
* Re: [patch iproute2 0/6] iproute2: add changes for switchdev
From: Jiri Pirko @ 2014-12-04 16:04 UTC (permalink / raw)
To: Roopa Prabhu
Cc: netdev, davem, nhorman, andy, tgraf, dborkman, ogerlitz, jesse,
pshelar, azhou, ben, stephen, jeffrey.t.kirsher, vyasevic,
xiyou.wangcong, john.r.fastabend, edumazet, jhs, 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, hemal
In-Reply-To: <54807398.40201@cumulusnetworks.com>
Thu, Dec 04, 2014 at 03:45:44PM CET, roopa@cumulusnetworks.com wrote:
>On 12/4/14, 6:34 AM, Jiri Pirko wrote:
>>Thu, Dec 04, 2014 at 03:26:50PM CET, roopa@cumulusnetworks.com wrote:
>>>On 12/4/14, 12:57 AM, Jiri Pirko wrote:
>>>>Jiri Pirko (1):
>>>> iproute2: ipa: show switch id
>>>>
>>>>Scott Feldman (5):
>>>> bridge/fdb: fix statistics output spacing
>>>> bridge/fdb: add flag/indication for FDB entry synced from offload
>>>> device
>>>> bridge/link: add new offload hwmode swdev
>>>Ack to most patches but nack on this one. The todo list still has a note to
>>>revist the flag to indicate switchdev offloads.
>>>Exposing this to userspace does not help that.
>>Hmm, note that this is already exposed to userspace, this patchset is
>>for iproute2 (userspace tool).
>hmmm, all feedback on the switchdev patches seemed to indicate we can change
>this later.
> I don't see swdev mode being used in the kernel anywhere today.
Well, it is, in rocker:
$ git grep BRIDGE_MODE_SWDEV
drivers/net/ethernet/rocker/rocker.c: if (mode != BRIDGE_MODE_SWDEV)
drivers/net/ethernet/rocker/rocker.c: u16 mode = BRIDGE_MODE_SWDEV;
include/uapi/linux/if_bridge.h:#define BRIDGE_MODE_SWDEV 2 /* Full switch device offload */
>I will send a patch to remove it. Its still in net-next and so can be changed
>?.
>I was going to resend my patch to introduce a common offload flag for all
>link objects.
>It would be nice if all of them had a consistent flag to indicate hw offload
>and iproute2 could display the same flag for all.
>Including bonds and vxlan's.
I do not understand the connection with BRIDGE_MODE_SWDEV. We discussed
this already. BRIDGE_MODE_SWDEV is a bridge mode, similar to for example
BRIDGE_MODE_VEPA and makes perfect sense to have it.
How vxlan and bonds come into the mixture, that is a puzzler for me.
Maybe I have to see patches.
>
>>
>>>> link: add missing IFLA_BRPORT_PROXYARP
>>>> bridge/link: add learning_sync policy flag
>>>>
>>>> bridge/fdb.c | 4 +++-
>>>> bridge/link.c | 17 +++++++++++++++--
>>>> include/linux/if_bridge.h | 1 +
>>>> include/linux/if_link.h | 3 +++
>>>> include/linux/neighbour.h | 1 +
>>>> ip/ipaddress.c | 8 ++++++++
>>>> man/man8/bridge.8 | 19 ++++++++++++++-----
>>>> 7 files changed, 45 insertions(+), 8 deletions(-)
>>>>
>
>--
>To unsubscribe from this list: send the line "unsubscribe netdev" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: Where exactly will arch_fast_hash be used
From: Herbert Xu @ 2014-12-04 16:10 UTC (permalink / raw)
To: David Laight
Cc: Daniel Borkmann, Thomas Graf, David S. Miller, Theodore Ts'o,
netdev@vger.kernel.org, Linux Kernel Mailing List
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D1CA0284A@AcuExch.aculab.com>
On Thu, Dec 04, 2014 at 03:56:30PM +0000, David Laight wrote:
>
> "This CRC algorithm used by this hash is 'linear', ie hash(a xor b) ==
> hash(a) xor hash(b). This means that it is relatively easy for a remote
> attacker to generate multiple items with the same hash."
The attacker could be local too, e.g., opening a netlink socket
can be done by any user and gets hashed in net/netlink/af_netlink.c.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [patch iproute2 1/6] iproute2: ipa: show switch id
From: Eric W. Biederman @ 2014-12-04 16:15 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, jhs, sfeldma,
f.fainelli, roopa, linville, jasowang, nicolas.dichtel,
ryazanov.s.a, buytenh, aviadr, nbd, alexei.starovoitov,
Neil.Jerram, ronye, simon.horman, alexander.h.duyck, john.ronciak,
mleitner, shrijeet, gospo, bcrl, hemal
In-Reply-To: <1417683438-10935-2-git-send-email-jiri@resnulli.us>
Jiri Pirko <jiri@resnulli.us> writes:
Would someone please explain to me what a switch id is?
I looked in the kernel source, and I looked here and while I know
switches I don't have a clue what a switch id is.
My primary concern at this point is that you have introduced a global
identifier that is isn't a hardware property (it certainly does not look
like a mac address) and that is unique across network namespaces and
thus breaks checkpoint/restart (aka CRIU).
Also what in the world does PHYS mean in IFLA_PHYS_SWITCH_ID? Does that
mean we can't have a purely software implementation of this interface?
Given that we will want a software implementation at some point
including PHYS in the name seems completely wrong.
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
> ---
> include/linux/if_link.h | 1 +
> ip/ipaddress.c | 8 ++++++++
> 2 files changed, 9 insertions(+)
>
> diff --git a/include/linux/if_link.h b/include/linux/if_link.h
> index 4732063..a6e2594 100644
> --- a/include/linux/if_link.h
> +++ b/include/linux/if_link.h
> @@ -145,6 +145,7 @@ enum {
> IFLA_CARRIER,
> IFLA_PHYS_PORT_ID,
> IFLA_CARRIER_CHANGES,
> + IFLA_PHYS_SWITCH_ID,
> __IFLA_MAX
> };
>
> diff --git a/ip/ipaddress.c b/ip/ipaddress.c
> index 4d99324..bd36a07 100644
> --- a/ip/ipaddress.c
> +++ b/ip/ipaddress.c
> @@ -589,6 +589,14 @@ int print_linkinfo(const struct sockaddr_nl *who,
> b1, sizeof(b1)));
> }
>
> + if (tb[IFLA_PHYS_SWITCH_ID]) {
> + SPRINT_BUF(b1);
> + fprintf(fp, "switchid %s ",
> + hexstring_n2a(RTA_DATA(tb[IFLA_PHYS_SWITCH_ID]),
> + RTA_PAYLOAD(tb[IFLA_PHYS_SWITCH_ID]),
> + b1, sizeof(b1)));
> + }
> +
> if (tb[IFLA_OPERSTATE])
> print_operstate(fp, rta_getattr_u8(tb[IFLA_OPERSTATE]));
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).