* Re: PHYless ethernet switch MAC-MAC serdes connection
From: Vijay @ 2015-01-21 7:09 UTC (permalink / raw)
To: netdev
In-Reply-To: <54BE9B0B.7070507@gmail.com>
Florian Fainelli <f.fainelli <at> gmail.com> writes:
>
> Do lan1 and lan2 interfaces set the RUNNING flag, which would indicate
> that the switch driver has detected a carrier for these individual
ports?
>
> Is the DSA switch driver correctly polling for LAN ports link
statuses?
>
Yes RUNNING flag is set.
Whenever I unplug/plug Ethernet cable. I see notification from dsa about
link up/down correctly.
>
> DSA registers a slave MDIO bus interface which calls into the "real"
> MDIO bus hardware to talk to the switch. In your case, you have both
an
> external switch and an external PHY connected to the same MDIO bus,
> which is fine.
>
> Your DT looks sane, although I suspect that lan2's PHY address is 3,
and
> not 2, as address 0 is typically a special MDIO broadcast address, so
> Port 0's PHY can be read at PHY addr 1, Port 1's PHY can be read at
PHY
> addr 2 etc... If you remove "lan2", does it work slightly better?
>
No, lan 1 and lan2 addresses are 0 and 1 respectively. I have logged
these values and these are printed on console as:
DSA: dsa.c: Init slave bus
DSA: slave.c: dsa_slave_mii_bus_init sw addr =0x1f , slave_mii_bus_id =
0x0
DSA: dsa.c: register created slave_mii_bus
Debug: drivers/net/phy/phy_device.c: Creating PHY-> phy_id = 0x1410c89,
addr = 0x0
Creating PHY-> AUTONEG_ENABLE for phy_id = 0x1410c89, addr = 0x0
Debug: drivers/net/phy/phy_device.c: Creating PHY-> phy_id = 0x1410c89,
addr = 0x1
Creating PHY-> AUTONEG_ENABLE for phy_id = 0x1410c89, addr = 0x1
libphy: dsa slave smi: probed
net lan1: PHY already attached
DSA: slave.c: mii_bus-> name = dsa slave smi , id =dsa-0:1f
net lan2: PHY already attached
DSA: slave.c: mii_bus-> name = dsa slave smi , id =dsa-0:1f
My eth0 fixed-link address is also 0 and eth1 PHY address is 1.
Removing lan2 is not improving the situation.
Even if I do not set up eth1. Then same issue is happening between lan1
and lan2. Whichever is configured first ( using ifconfig) is able to
transmit packet (Tx increments). And rest all ports ( lan2) are just
receiving (their Tx counters remain 0). Even if I ping to lan2 ip,
response will be received only when ethernet cable is plugged to lan1 (
because it will only be able transmit the response), It does not matter
if cable is plugged to lan2 port.
^ permalink raw reply
* [PATCH net] net: ipv6: allow explicitly choosing optimistic addresses
From: Erik Kline @ 2015-01-21 7:02 UTC (permalink / raw)
To: netdev; +Cc: hannes, davem, Erik Kline
RFC 4429 ("Optimistic DAD") states that optimistic addresses
should be treated as deprecated addresses. From section 2.1:
Unless noted otherwise, components of the IPv6 protocol stack
should treat addresses in the Optimistic state equivalently to
those in the Deprecated state, indicating that the address is
available for use but should not be used if another suitable
address is available.
Optimistic addresses are indeed avoided when other addresses are
available (i.e. at source address selection time), but they have
not heretofore been available for things like explicit bind() and
sendmsg() with struct in6_pktinfo, etc.
This change makes optimistic addresses treated more like
deprecated addresses than tentative ones.
Signed-off-by: Erik Kline <ek@google.com>
---
include/net/addrconf.h | 3 +++
net/ipv6/addrconf.c | 14 ++++++++++++--
net/ipv6/ndisc.c | 3 ++-
3 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/include/net/addrconf.h b/include/net/addrconf.h
index d13573b..80456f7 100644
--- a/include/net/addrconf.h
+++ b/include/net/addrconf.h
@@ -62,6 +62,9 @@ int addrconf_set_dstaddr(struct net *net, void __user *arg);
int ipv6_chk_addr(struct net *net, const struct in6_addr *addr,
const struct net_device *dev, int strict);
+int ipv6_chk_addr_and_flags(struct net *net, const struct in6_addr *addr,
+ const struct net_device *dev, int strict,
+ u32 banned_flags);
#if defined(CONFIG_IPV6_MIP6) || defined(CONFIG_IPV6_MIP6_MODULE)
int ipv6_chk_home_addr(struct net *net, const struct in6_addr *addr);
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index f7c8bbe..422976b 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -1519,6 +1519,14 @@ static int ipv6_count_addresses(struct inet6_dev *idev)
int ipv6_chk_addr(struct net *net, const struct in6_addr *addr,
const struct net_device *dev, int strict)
{
+ return ipv6_chk_addr_and_flags(net, addr, dev, strict, 0);
+}
+EXPORT_SYMBOL(ipv6_chk_addr);
+
+int ipv6_chk_addr_and_flags(struct net *net, const struct in6_addr *addr,
+ const struct net_device *dev, int strict,
+ u32 banned_flags)
+{
struct inet6_ifaddr *ifp;
unsigned int hash = inet6_addr_hash(addr);
@@ -1527,7 +1535,9 @@ int ipv6_chk_addr(struct net *net, const struct in6_addr *addr,
if (!net_eq(dev_net(ifp->idev->dev), net))
continue;
if (ipv6_addr_equal(&ifp->addr, addr) &&
- !(ifp->flags&IFA_F_TENTATIVE) &&
+ (!(ifp->flags&IFA_F_TENTATIVE) ||
+ ifp->flags&IFA_F_OPTIMISTIC) &&
+ !(ifp->flags&banned_flags) &&
(dev == NULL || ifp->idev->dev == dev ||
!(ifp->scope&(IFA_LINK|IFA_HOST) || strict))) {
rcu_read_unlock_bh();
@@ -1538,7 +1548,7 @@ int ipv6_chk_addr(struct net *net, const struct in6_addr *addr,
rcu_read_unlock_bh();
return 0;
}
-EXPORT_SYMBOL(ipv6_chk_addr);
+EXPORT_SYMBOL(ipv6_chk_addr_and_flags);
static bool ipv6_chk_same_addr(struct net *net, const struct in6_addr *addr,
struct net_device *dev)
diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index 6828667..a726c7a 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -655,7 +655,8 @@ static void ndisc_solicit(struct neighbour *neigh, struct sk_buff *skb)
struct in6_addr *target = (struct in6_addr *)&neigh->primary_key;
int probes = atomic_read(&neigh->probes);
- if (skb && ipv6_chk_addr(dev_net(dev), &ipv6_hdr(skb)->saddr, dev, 1))
+ if (skb && ipv6_chk_addr_and_flags(dev_net(dev), &ipv6_hdr(skb)->saddr,
+ dev, 1, IFA_F_OPTIMISTIC))
saddr = &ipv6_hdr(skb)->saddr;
probes -= NEIGH_VAR(neigh->parms, UCAST_PROBES);
if (probes < 0) {
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply related
* Re: [PATCH] net: dsa: set parent of hwmon device
From: Florian Fainelli @ 2015-01-21 5:58 UTC (permalink / raw)
To: Guenter Roeck
Cc: Vivien Didelot, netdev, David S . Miller,
linux-kernel@vger.kernel.org, kernel
In-Reply-To: <54BF221E.8000409@roeck-us.net>
2015-01-20 19:50 GMT-08:00 Guenter Roeck <linux@roeck-us.net>:
> On 01/20/2015 04:13 PM, Vivien Didelot wrote:
>>
>> Set the dsa device as the parent of the hwmon device, in order to link
>> the hwmon subsystem under the corresponding /sys/devices/platform/dsa.X/
>> sysfs directory.
>>
>> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
>> ---
>> net/dsa/dsa.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
>> index 3731714..363102a 100644
>> --- a/net/dsa/dsa.c
>> +++ b/net/dsa/dsa.c
>> @@ -347,7 +347,7 @@ dsa_switch_setup(struct dsa_switch_tree *dst, int
>> index,
>> hname[j] = '\0';
>> scnprintf(ds->hwmon_name, sizeof(ds->hwmon_name),
>> "%s_dsa%d",
>> hname, index);
>> - ds->hwmon_dev = hwmon_device_register_with_groups(NULL,
>> + ds->hwmon_dev = hwmon_device_register_with_groups(parent,
>> ds->hwmon_name, ds,
>> dsa_hwmon_groups);
>> if (IS_ERR(ds->hwmon_dev))
>> ds->hwmon_dev = NULL;
>>
>
> Looking into my old e-mail, turns out we did not add the parent device
> because
> it affected the output of the "sensors" command, and we wanted the device
> to be handled as 'virtual device' (which implies no parent). That was an
> explicit
> part of the patch set (v2 of 'net: dsa: Add support for reporting switch
> chip
> temperatures'), compared to v1, which did set the parent device.
>
> I would suggest to keep the code as is.
Maybe follow-up with a comment adding that above the call to
hwmon_device_register_with_groups()? I suspect the intent is clear if
you are deep into hwmon devices, but not necessarily for the reader ;)
>
> Thanks,
> Guenter
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
Florian
^ permalink raw reply
* Re: [PATCH 7/9] rhashtable: Per bucket locks & deferred expansion/shrinking
From: Paul E. McKenney @ 2015-01-21 5:36 UTC (permalink / raw)
To: Herbert Xu
Cc: Pablo Neira Ayuso, Patrick McHardy, Thomas Graf, David Laight,
davem@davemloft.net, netdev@vger.kernel.org, edumazet@google.com,
john.r.fastabend@intel.com, josh@joshtriplett.org,
netfilter-devel@vger.kernel.org
In-Reply-To: <20150121053025.GA23481@gondor.apana.org.au>
On Wed, Jan 21, 2015 at 04:30:25PM +1100, Herbert Xu wrote:
> On Tue, Jan 20, 2015 at 09:29:07PM -0800, Paul E. McKenney wrote:
> >
> > Well, you -could- batch them up, so that a single snapshot covered
> > several users, and once that set was done and memory reclaimed, a second
> > snapshot could cover any additional users that requested dumps/walks in
> > the meantime. Or are users allowed to walk arbitrarily slowly through
> > the table?
>
> Yes they can. You could CTRL-Z a dumper quite easily.
OK, never mind, then! ;-)
Thanx, Paul
^ permalink raw reply
* Re: [PATCH 7/9] rhashtable: Per bucket locks & deferred expansion/shrinking
From: Herbert Xu @ 2015-01-21 5:30 UTC (permalink / raw)
To: Paul E. McKenney
Cc: Pablo Neira Ayuso, Patrick McHardy, Thomas Graf, David Laight,
davem@davemloft.net, netdev@vger.kernel.org, edumazet@google.com,
john.r.fastabend@intel.com, josh@joshtriplett.org,
netfilter-devel@vger.kernel.org
In-Reply-To: <20150121052907.GY9719@linux.vnet.ibm.com>
On Tue, Jan 20, 2015 at 09:29:07PM -0800, Paul E. McKenney wrote:
>
> Well, you -could- batch them up, so that a single snapshot covered
> several users, and once that set was done and memory reclaimed, a second
> snapshot could cover any additional users that requested dumps/walks in
> the meantime. Or are users allowed to walk arbitrarily slowly through
> the table?
Yes they can. You could CTRL-Z a dumper quite easily.
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 7/9] rhashtable: Per bucket locks & deferred expansion/shrinking
From: Paul E. McKenney @ 2015-01-21 5:29 UTC (permalink / raw)
To: Herbert Xu
Cc: Pablo Neira Ayuso, Patrick McHardy, Thomas Graf, David Laight,
davem@davemloft.net, netdev@vger.kernel.org, edumazet@google.com,
john.r.fastabend@intel.com, josh@joshtriplett.org,
netfilter-devel@vger.kernel.org
In-Reply-To: <20150121052333.GA23382@gondor.apana.org.au>
On Wed, Jan 21, 2015 at 04:23:33PM +1100, Herbert Xu wrote:
> On Mon, Jan 19, 2015 at 01:01:21AM -0800, Paul E. McKenney wrote:
> >
> > One unconventional way of handling this is to associate the scan with
> > a one-to-one resize operation. This can be implemented to have the
> > effect of taking a snapshot of the table.
>
> The problem is that in general (not for netfilter, but certainly
> other rhashtable users such as netlink) dumps/walks can be started
> by ordinary users and I don't think we can afford creating a
> snapshot for each one, or can we?
Well, you -could- batch them up, so that a single snapshot covered
several users, and once that set was done and memory reclaimed, a second
snapshot could cover any additional users that requested dumps/walks in
the meantime. Or are users allowed to walk arbitrarily slowly through
the table?
Thanx, Paul
^ permalink raw reply
* Re: [PATCH net-next 1/5] bonding: keep bond interface carrier off until at least one active member
From: Jonathan Toppins @ 2015-01-21 5:27 UTC (permalink / raw)
To: Scott Feldman; +Cc: Netdev, Andy Gospodarek
In-Reply-To: <CAE4R7bBzeO5MvL5zS5Piq6Ld2ZbD8smGx8XaJy5SvY7kHbX_Kw@mail.gmail.com>
On 1/19/15 5:15 PM, Scott Feldman wrote:
>
> On Jan 16, 2015 7:57 AM, "Jonathan Toppins"
> <jtoppins@cumulusnetworks.com <mailto:jtoppins@cumulusnetworks.com>> wrote:
> >
> > From: Scott Feldman <sfeldma@cumulusnetworks.com
> <mailto:sfeldma@cumulusnetworks.com>>
>
> Hmmmm, this feels a little creepy, sending a patch from my past. I do
> appreciate the attribution, but I can't ACK or NACK or even comment as
> I've lost the context around this patch. And I'm afraid I'll break out
> in hives if I look at the bonding code again, so if you don't mind,
> please remove me from this patch. Thank you.
>
Have removed you for v2 of series, should we consider this a blanket
statement for any other patches we might be releasing that you had a
hand in? There were a few ;)
^ permalink raw reply
* Re: [PATCH 7/9] rhashtable: Per bucket locks & deferred expansion/shrinking
From: Herbert Xu @ 2015-01-21 5:23 UTC (permalink / raw)
To: Paul E. McKenney
Cc: Pablo Neira Ayuso, Patrick McHardy, Thomas Graf, David Laight,
davem@davemloft.net, netdev@vger.kernel.org, edumazet@google.com,
john.r.fastabend@intel.com, josh@joshtriplett.org,
netfilter-devel@vger.kernel.org
In-Reply-To: <20150119090121.GG9719@linux.vnet.ibm.com>
On Mon, Jan 19, 2015 at 01:01:21AM -0800, Paul E. McKenney wrote:
>
> One unconventional way of handling this is to associate the scan with
> a one-to-one resize operation. This can be implemented to have the
> effect of taking a snapshot of the table.
The problem is that in general (not for netfilter, but certainly
other rhashtable users such as netlink) dumps/walks can be started
by ordinary users and I don't think we can afford creating a
snapshot for each one, or can we?
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 net-next 1/5] bonding: keep bond interface carrier off until at least one active member
From: Jonathan Toppins @ 2015-01-21 5:22 UTC (permalink / raw)
To: Jay Vosburgh
Cc: netdev, Scott Feldman, Andy Gospodarek, Veaceslav Falico,
Nikolay Aleksandrov
In-Reply-To: <20956.1421702213@famine>
On 1/19/15 4:16 PM, Jay Vosburgh wrote:
> Jonathan Toppins <jtoppins@cumulusnetworks.com> wrote:
>
>> From: Scott Feldman <sfeldma@cumulusnetworks.com>
>>
>> Bonding driver parameter min_links is now used to signal upper-level
>> protocols of bond status. The way it works is if the total number of
>> active members in slaves drops below min_links, the bond link carrier
>> will go down, signaling upper levels that bond is inactive. When active
>> members returns to >= min_links, bond link carrier will go up (RUNNING),
>> and protocols can resume. When bond is carrier down, member ports are
>> in stp fwd state blocked (rather than normal disabled state), so
>> low-level ctrl protocols (LACP) can still get in and be processed by
>> bonding driver.
>
> Presuming that "stp" is Spanning Tree, is the last sentence
> above actually describing the behavior of a bridge port when a bond is
> the member of the bridge? I'm not sure I understand what "member ports"
> refers to (bridge ports or bonding slaves).
Ack, maybe replacing the last sentence with something like:
When bond is carrier down, the slave ports are only forwarding
low-level control protocols (e.g. LACP PDU) and discarding all other
packets.
>> @@ -2381,10 +2386,15 @@ int bond_3ad_set_carrier(struct bonding *bond)
>> ret = 0;
>> goto out;
>> }
>> +
>> + bond_for_each_slave_rcu(bond, slave, iter)
>> + if (SLAVE_AD_INFO(slave)->aggregator.is_active)
>> + active_slaves++;
>> +
>> active = __get_active_agg(&(SLAVE_AD_INFO(first_slave)->aggregator));
>> - if (active) {
>> + if (active && __agg_has_partner(active)) {
>
> Why "__agg_has_partner"? Since the "else" of this clause is:
>
> } else if (netif_carrier_ok(bond->dev)) {
> netif_carrier_off(bond->dev);
> }
>
> I'm wondering if this will do the right thing for the case that
> there are no LACP partners at all (e.g., the switch ports do not have
> LACP enabled), in which case the active aggregator should be a single
> "individual" port as a fallback, but will not have a partner.
>
> -J
>
I see your point. The initial thinking was the logical bond carrier
should not be brought up until the bond has a partner and is ready to
pass traffic, otherwise we start blackholing frames. Looking over the
code it seems the aggregator.is_individual flag is only set to true when
a slave is in half-duplex, this seems odd?
My initial thinking to alleviate the concern is something like the
following:
if (active && !SLAVE_AD_INFO(slave)->aggregator.is_individual &&
__agg_has_partner(active)) {
/* set carrier based on min_links */
} else if (active && SLAVE_AD_INFO(slave)->aggregator.is_individual) {
/* set bond carrier state according to carrier state of slave */
} else if (netif_carrier_ok(bond->dev)) {
netif_carrier_off(bond->dev);
}
Maybe I am missing something and there is a simpler option.
Thinking about how to validate this, it seems having a bond with two
slaves and both slaves in half-duplex will force an aggregator that is
individual to be selected.
Thoughts?
-Jon
^ permalink raw reply
* Re: [PATCH 3/3] netlink: Lock out table resizes while dumping Netlink sockets
From: Herbert Xu @ 2015-01-21 5:15 UTC (permalink / raw)
To: Thomas Graf
Cc: Patrick McHardy, davem, paulmck, ying.xue, netdev,
netfilter-devel
In-Reply-To: <20150121050819.GA23062@gondor.apana.org.au>
On Wed, Jan 21, 2015 at 04:08:19PM +1100, Herbert Xu wrote:
>
> OK I think I have a solution for you guys. But first you'll need to
> wait for me to undo the nulls stuff so I can steal that bit which
> is central to my solution.
>
> Essentially I need a bit to indicate an entry in the bucket chain
> should be skipped, either because it has just been removed or that
> it is a walker entry (see xfrm_state_walk).
>
> The way it'll work then is exactly the same as xfrm_state_walk,
> except that the linked list is broken up into individual buckets.
>
> Of course we'll still need to postpone resizes (and rehashes which
> is what my work is about) during a walk but I think that's a fair
> price to pay.
I failed to address the security aspect of this approach. Obviously
this can only work if the only entites doing the walk are trusted.
Which means that the vast majority (if not all) hash table users
would be excluded, in particular, netlink.
Even with trusted cases such as netfilter I'm uneasy about this
so I don't think I'm going to go forward with this approach.
At this point I think the best thing that'll give you stable dumps
is a separate list like xfrm_state_walk. If you did that then
we don't need to touch rhashtable at all.
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 3/3] netlink: Lock out table resizes while dumping Netlink sockets
From: Herbert Xu @ 2015-01-21 5:11 UTC (permalink / raw)
To: Patrick McHardy
Cc: Thomas Graf, davem, paulmck, ying.xue, netdev, netfilter-devel
In-Reply-To: <20150120143154.GR14883@acer.localdomain>
On Tue, Jan 20, 2015 at 02:31:54PM +0000, Patrick McHardy wrote:
>
> An alternative would be to set a flag in ht when a dump begins that
> indicates to skip resizing operations and on the end of the dump
> perform any resizing operations that might be necessary. Herbert
> disagrees though and he might be right.
It's a bit more complex than just a disagreement :)
See the email I just sent. But the crucial thing is that postponed
resizing/rehashing during dumping is only possible if the only
entity that can perform dumping is trusted. Otherwise the whole
thing falls apart really quickly.
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 3/3] netlink: Lock out table resizes while dumping Netlink sockets
From: Herbert Xu @ 2015-01-21 5:08 UTC (permalink / raw)
To: Thomas Graf
Cc: Patrick McHardy, davem, paulmck, ying.xue, netdev,
netfilter-devel
In-Reply-To: <20150120153556.GJ20315@casper.infradead.org>
On Tue, Jan 20, 2015 at 03:35:56PM +0000, Thomas Graf wrote:
> On 01/20/15 at 03:21pm, Patrick McHardy wrote:
> > I think its preferrable to make the need to handle NETLINK_F_DUMP_INTR
> > as noticable as possible and not hide it. Silent failure is the worst
> > kind of failure.
>
> I agree to that. The point here is to avoid unnecessary use of
> NETLINK_F_DUMP_INTR if all entries fit into a single message buffer.
OK I think I have a solution for you guys. But first you'll need to
wait for me to undo the nulls stuff so I can steal that bit which
is central to my solution.
Essentially I need a bit to indicate an entry in the bucket chain
should be skipped, either because it has just been removed or that
it is a walker entry (see xfrm_state_walk).
The way it'll work then is exactly the same as xfrm_state_walk,
except that the linked list is broken up into individual buckets.
Of course we'll still need to postpone resizes (and rehashes which
is what my work is about) during a walk but I think that's a fair
price to pay.
This also means handling insertion failures but I think that
should be acceptable if we make it based on a configurable maximum
chain length along with forced resize/rehash where possible.
Note that this can be made optional, i.e., if the user can afford
memory to do their own walking (e.g., xfrm_state), then none of
this needs to happen and it'll just work as it does now. IOW if
you don't use this special rhashtable walk function then you're
not affected.
Thoughts?
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] net: dsa: set parent of hwmon device
From: Guenter Roeck @ 2015-01-21 3:50 UTC (permalink / raw)
To: Vivien Didelot, netdev; +Cc: David S . Miller, linux-kernel, kernel
In-Reply-To: <1421799212-2028-1-git-send-email-vivien.didelot@savoirfairelinux.com>
On 01/20/2015 04:13 PM, Vivien Didelot wrote:
> Set the dsa device as the parent of the hwmon device, in order to link
> the hwmon subsystem under the corresponding /sys/devices/platform/dsa.X/
> sysfs directory.
>
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> ---
> net/dsa/dsa.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
> index 3731714..363102a 100644
> --- a/net/dsa/dsa.c
> +++ b/net/dsa/dsa.c
> @@ -347,7 +347,7 @@ dsa_switch_setup(struct dsa_switch_tree *dst, int index,
> hname[j] = '\0';
> scnprintf(ds->hwmon_name, sizeof(ds->hwmon_name), "%s_dsa%d",
> hname, index);
> - ds->hwmon_dev = hwmon_device_register_with_groups(NULL,
> + ds->hwmon_dev = hwmon_device_register_with_groups(parent,
> ds->hwmon_name, ds, dsa_hwmon_groups);
> if (IS_ERR(ds->hwmon_dev))
> ds->hwmon_dev = NULL;
>
Looking into my old e-mail, turns out we did not add the parent device because
it affected the output of the "sensors" command, and we wanted the device
to be handled as 'virtual device' (which implies no parent). That was an explicit
part of the patch set (v2 of 'net: dsa: Add support for reporting switch chip
temperatures'), compared to v1, which did set the parent device.
I would suggest to keep the code as is.
Thanks,
Guenter
^ permalink raw reply
* [PATCH net] ipv6: Fix __ip6_route_redirect
From: Martin KaFai Lau @ 2015-01-21 3:16 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Hannes Frederic Sowa, kernel-team
In my last commit (a3c00e4: ipv6: Remove BACKTRACK macro), the changes in
__ip6_route_redirect is incorrect. The following case is missed:
1. The for loop tries to find a valid gateway rt. If it fails to find
one, rt will be NULL.
2. When rt is NULL, it is set to the ip6_null_entry.
3. The newly added 'else if', from a3c00e4, will stop the backtrack from
happening.
Signed-off-by: Martin KaFai Lau <kafai@fb.com>
---
net/ipv6/route.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 166e33b..49596535 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -1242,12 +1242,16 @@ restart:
rt = net->ipv6.ip6_null_entry;
else if (rt->dst.error) {
rt = net->ipv6.ip6_null_entry;
- } else if (rt == net->ipv6.ip6_null_entry) {
+ goto out;
+ }
+
+ if (rt == net->ipv6.ip6_null_entry) {
fn = fib6_backtrack(fn, &fl6->saddr);
if (fn)
goto restart;
}
+out:
dst_hold(&rt->dst);
read_unlock_bh(&table->tb6_lock);
--
1.8.1
^ permalink raw reply related
* Re: [PATCH] net: dsa: set parent of hwmon device
From: Guenter Roeck @ 2015-01-21 2:48 UTC (permalink / raw)
To: Vivien Didelot, netdev; +Cc: David S . Miller, linux-kernel, kernel
In-Reply-To: <1421799212-2028-1-git-send-email-vivien.didelot@savoirfairelinux.com>
On 01/20/2015 04:13 PM, Vivien Didelot wrote:
> Set the dsa device as the parent of the hwmon device, in order to link
> the hwmon subsystem under the corresponding /sys/devices/platform/dsa.X/
> sysfs directory.
>
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> ---
> net/dsa/dsa.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
> index 3731714..363102a 100644
> --- a/net/dsa/dsa.c
> +++ b/net/dsa/dsa.c
> @@ -347,7 +347,7 @@ dsa_switch_setup(struct dsa_switch_tree *dst, int index,
> hname[j] = '\0';
> scnprintf(ds->hwmon_name, sizeof(ds->hwmon_name), "%s_dsa%d",
> hname, index);
> - ds->hwmon_dev = hwmon_device_register_with_groups(NULL,
> + ds->hwmon_dev = hwmon_device_register_with_groups(parent,
> ds->hwmon_name, ds, dsa_hwmon_groups);
> if (IS_ERR(ds->hwmon_dev))
> ds->hwmon_dev = NULL;
>
I'll have to look into this again; not sure if this works because there is
no 1:1 relationship between dsa device and hwmon device (or switch chip).
Guenter
^ permalink raw reply
* RE: [E1000-devel] [PATCH 1/2] if_link: Add VF multicast promiscuous mode control
From: Skidmore, Donald C @ 2015-01-21 1:30 UTC (permalink / raw)
To: Hiroshi Shimamoto, Bjørn Mork
Cc: e1000-devel@lists.sourceforge.net, netdev@vger.kernel.org,
Choi, Sy Jong, linux-kernel@vger.kernel.org, Hayato Momma
In-Reply-To: <7F861DC0615E0C47A872E6F3C5FCDDBD05E07F09@BPXM14GP.gisp.nec.co.jp>
> -----Original Message-----
> From: Hiroshi Shimamoto [mailto:h-shimamoto@ct.jp.nec.com]
> Sent: Tuesday, January 20, 2015 5:07 PM
> To: Skidmore, Donald C; Bjørn Mork
> Cc: e1000-devel@lists.sourceforge.net; netdev@vger.kernel.org; Choi, Sy
> Jong; linux-kernel@vger.kernel.org; Hayato Momma
> Subject: RE: [E1000-devel] [PATCH 1/2] if_link: Add VF multicast promiscuous
> mode control
>
> > Subject: RE: [E1000-devel] [PATCH 1/2] if_link: Add VF multicast
> > promiscuous mode control
> >
> >
> >
> > > -----Original Message-----
> > > From: Hiroshi Shimamoto [mailto:h-shimamoto@ct.jp.nec.com]
> > > Sent: Tuesday, January 20, 2015 3:40 PM
> > > To: Bjørn Mork
> > > Cc: e1000-devel@lists.sourceforge.net; netdev@vger.kernel.org; Choi,
> > > Sy Jong; linux-kernel@vger.kernel.org; Hayato Momma
> > > Subject: Re: [E1000-devel] [PATCH 1/2] if_link: Add VF multicast
> > > promiscuous mode control
> > >
> > > > Subject: Re: [PATCH 1/2] if_link: Add VF multicast promiscuous
> > > > mode control
> > > >
> > > > Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com> writes:
> > > >
> > > > > From: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
> > > > >
> > > > > Add netlink directives and ndo entry to control VF multicast
> > > > > promiscuous
> > > mode.
> > > > >
> > > > > Intel ixgbe and ixgbevf driver can handle only 30 multicast MAC
> > > > > addresses per VF. It means that we cannot assign over 30 IPv6
> > > > > addresses to a single VF interface on VM. We want thousands IPv6
> > > addresses in VM.
> > > > >
> > > > > There is capability of multicast promiscuous mode in Intel 82599 chip.
> > > > > It enables all multicast packets are delivered to the target VF.
> > > > >
> > > > > This patch prepares to control that VF multicast promiscuous
> > > functionality.
> > > >
> > > > Adding a new hook for this seems over-complicated to me. And it
> > > > still doesn't solve the real problems that
> > > > a) the user has to know about this limit, and
> > > > b) manually configure the feature
> > > >
> > > > Most of us, lacking the ability to imagine such arbitrary hardware
> > > > limitations, will go through a few hours of frustrating debugging
> > > > before we figure this one out...
> > > >
> > > > Why can't the ixgbevf driver just automatically signal the ixgbe
> > > > driver to enable multicast promiscuous mode whenever the list
> > > > grows past the limit?
> > >
> > > I had submitted a patch to change ixgbe and ixgbevf driver for this issue.
> > > https://lkml.org/lkml/2014/11/27/269
> > >
> > > The previous patch introduces API between ixgbe and ixgbevf driver
> > > to enable multicast promiscuous mode, and ixgbevf enables it
> > > automatically if the number of addresses is over than 30.
> >
> > I believe the issue is with allowing a VF to automatically enter
> > Promiscuous Multicast without the PF's ok is concern
>
> So you mean that we should take care about enabling VF multicast
> promiscuous mode in host side, right? The host allows multicast promiscuous
> and VF requests it too, then enables VF multicast promiscuous mode.
> So, what is preferred way to do in host do you think?
I think we are saying the same thing. I believe it would be fine if the VF requests this to happen (threw a mailbox message like you set up) and the PF will do it if the systems policy has been set up that way (as you did with the control mode).
This way the behavior (related to multicast) is the same as it has been, unless the system has been setup specifically to allow VF multicast promiscuous mode.
I know it's been mentioned that this is onerous on those who want this behavior to be automatic, but I don't see how else it could be done and take account for people that are concerned about allowing a (possibly untrusted) VM promoting itself in to multicast promiscuous mode.
>
> > over VM isolation. Of course that isolation, when it comes to multicast, is
> rather limited anyway given that our multicast
> > filter uses only 12-bit of the address for a match. Still this (or
> > doing it by default) would only open that up considerably more (all
> > multicasts). I assume for your application you're not concerned, but are
> there other use cases that would worry about such things?
>
> Sorry I couldn't catch the point.
>
> What is the issue? I think there is no difference for the users who don't want
> many multicast addresses in guest. In the current implementation,
> overflowed multicast addresses silently discarded in ixgbevf. I believe there
> is no user who want to use over 30 multicast addresses now. If VF multicast
> promiscuous mode is enabled in certain VF, the behavior of other VFs is not
> changed.
>
> thanks,
> Hiroshi
>
> >
> > Thanks,
> > -Don Skidmore <donald.c.skidmore@intel.com>
> >
> > >
> > > I got some comment and I would like to clarify the point, but there
> > > was no answer.
> > > That's the reason I submitted this patch.
> > >
> > > Do you think a patch for the ixgbe/ixgbevf driver is preferred?
> > >
> > >
> > > thanks,
> > > Hiroshi
> > >
> > > >
> > > > I'd also like to note that this comment in
> > > > drivers/net/ethernet/intel/ixgbevf/vf.c
> > > > indicates that the author had some ideas about how more than 30
> > > > addresses could/should be handled:
> > > >
> > > > static s32 ixgbevf_update_mc_addr_list_vf(struct ixgbe_hw *hw,
> > > > struct net_device *netdev)
> > > > {
> > > > struct netdev_hw_addr *ha;
> > > > u32 msgbuf[IXGBE_VFMAILBOX_SIZE];
> > > > u16 *vector_list = (u16 *)&msgbuf[1];
> > > > u32 cnt, i;
> > > >
> > > > /* Each entry in the list uses 1 16 bit word. We have 30
> > > > * 16 bit words available in our HW msg buffer (minus 1 for the
> > > > * msg type). That's 30 hash values if we pack 'em right. If
> > > > * there are more than 30 MC addresses to add then punt the
> > > > * extras for now and then add code to handle more than 30 later.
> > > > * It would be unusual for a server to request that many multi-cast
> > > > * addresses except for in large enterprise network environments.
> > > > */
> > > >
> > > >
> > > >
> > > > The last 2 lines of that comment are of course totally bogus and
> > > > pointless and should be deleted in any case... It's obvious that
> > > > 30 multicast addresses is ridiculously low for lots of normal use cases.
> > > >
> > > >
> > > > Bjørn
^ permalink raw reply
* Re: [PATCH 1/2] if_link: Add VF multicast promiscuous mode control
From: Hiroshi Shimamoto @ 2015-01-21 1:07 UTC (permalink / raw)
To: Skidmore, Donald C, Bjørn Mork
Cc: e1000-devel@lists.sourceforge.net, netdev@vger.kernel.org,
Choi, Sy Jong, Hayato Momma, linux-kernel@vger.kernel.org
In-Reply-To: <F6FB0E698C9B3143BDF729DF222866469129B7F5@ORSMSX110.amr.corp.intel.com>
> Subject: RE: [E1000-devel] [PATCH 1/2] if_link: Add VF multicast promiscuous mode control
>
>
>
> > -----Original Message-----
> > From: Hiroshi Shimamoto [mailto:h-shimamoto@ct.jp.nec.com]
> > Sent: Tuesday, January 20, 2015 3:40 PM
> > To: Bjørn Mork
> > Cc: e1000-devel@lists.sourceforge.net; netdev@vger.kernel.org; Choi, Sy
> > Jong; linux-kernel@vger.kernel.org; Hayato Momma
> > Subject: Re: [E1000-devel] [PATCH 1/2] if_link: Add VF multicast promiscuous
> > mode control
> >
> > > Subject: Re: [PATCH 1/2] if_link: Add VF multicast promiscuous mode
> > > control
> > >
> > > Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com> writes:
> > >
> > > > From: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
> > > >
> > > > Add netlink directives and ndo entry to control VF multicast promiscuous
> > mode.
> > > >
> > > > Intel ixgbe and ixgbevf driver can handle only 30 multicast MAC
> > > > addresses per VF. It means that we cannot assign over 30 IPv6
> > > > addresses to a single VF interface on VM. We want thousands IPv6
> > addresses in VM.
> > > >
> > > > There is capability of multicast promiscuous mode in Intel 82599 chip.
> > > > It enables all multicast packets are delivered to the target VF.
> > > >
> > > > This patch prepares to control that VF multicast promiscuous
> > functionality.
> > >
> > > Adding a new hook for this seems over-complicated to me. And it still
> > > doesn't solve the real problems that
> > > a) the user has to know about this limit, and
> > > b) manually configure the feature
> > >
> > > Most of us, lacking the ability to imagine such arbitrary hardware
> > > limitations, will go through a few hours of frustrating debugging
> > > before we figure this one out...
> > >
> > > Why can't the ixgbevf driver just automatically signal the ixgbe
> > > driver to enable multicast promiscuous mode whenever the list grows
> > > past the limit?
> >
> > I had submitted a patch to change ixgbe and ixgbevf driver for this issue.
> > https://lkml.org/lkml/2014/11/27/269
> >
> > The previous patch introduces API between ixgbe and ixgbevf driver to
> > enable multicast promiscuous mode, and ixgbevf enables it automatically if
> > the number of addresses is over than 30.
>
> I believe the issue is with allowing a VF to automatically enter Promiscuous Multicast without the PF's ok is concern
So you mean that we should take care about enabling VF multicast promiscuous mode
in host side, right? The host allows multicast promiscuous and VF requests it too,
then enables VF multicast promiscuous mode.
So, what is preferred way to do in host do you think?
> over VM isolation. Of course that isolation, when it comes to multicast, is rather limited anyway given that our multicast
> filter uses only 12-bit of the address for a match. Still this (or doing it by default) would only open that up considerably
> more (all multicasts). I assume for your application you're not concerned, but are there other use cases that would worry
> about such things?
Sorry I couldn't catch the point.
What is the issue? I think there is no difference for the users who don't want
many multicast addresses in guest. In the current implementation, overflowed
multicast addresses silently discarded in ixgbevf. I believe there is no user
who want to use over 30 multicast addresses now. If VF multicast promiscuous mode
is enabled in certain VF, the behavior of other VFs is not changed.
thanks,
Hiroshi
>
> Thanks,
> -Don Skidmore <donald.c.skidmore@intel.com>
>
> >
> > I got some comment and I would like to clarify the point, but there was no
> > answer.
> > That's the reason I submitted this patch.
> >
> > Do you think a patch for the ixgbe/ixgbevf driver is preferred?
> >
> >
> > thanks,
> > Hiroshi
> >
> > >
> > > I'd also like to note that this comment in
> > > drivers/net/ethernet/intel/ixgbevf/vf.c
> > > indicates that the author had some ideas about how more than 30
> > > addresses could/should be handled:
> > >
> > > static s32 ixgbevf_update_mc_addr_list_vf(struct ixgbe_hw *hw,
> > > struct net_device *netdev)
> > > {
> > > struct netdev_hw_addr *ha;
> > > u32 msgbuf[IXGBE_VFMAILBOX_SIZE];
> > > u16 *vector_list = (u16 *)&msgbuf[1];
> > > u32 cnt, i;
> > >
> > > /* Each entry in the list uses 1 16 bit word. We have 30
> > > * 16 bit words available in our HW msg buffer (minus 1 for the
> > > * msg type). That's 30 hash values if we pack 'em right. If
> > > * there are more than 30 MC addresses to add then punt the
> > > * extras for now and then add code to handle more than 30 later.
> > > * It would be unusual for a server to request that many multi-cast
> > > * addresses except for in large enterprise network environments.
> > > */
> > >
> > >
> > >
> > > The last 2 lines of that comment are of course totally bogus and
> > > pointless and should be deleted in any case... It's obvious that 30
> > > multicast addresses is ridiculously low for lots of normal use cases.
> > >
> > >
> > > Bjørn
------------------------------------------------------------------------------
New Year. New Location. New Benefits. New Data Center in Ashburn, VA.
GigeNET is offering a free month of service with a new server in Ashburn.
Choose from 2 high performing configs, both with 100TB of bandwidth.
Higher redundancy.Lower latency.Increased capacity.Completely compliant.
http://p.sf.net/sfu/gigenet
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply
* [PATCH net-next 2/2] net: dsa: bcm_sf2: factor interrupt disabling in a function
From: Florian Fainelli @ 2015-01-21 0:42 UTC (permalink / raw)
To: netdev; +Cc: davem, Florian Fainelli
In-Reply-To: <1421800920-6281-1-git-send-email-f.fainelli@gmail.com>
Factor the interrupt disabling in a function: bcm_sf2_intr_disable()
since we are doing the same thing in the setup and suspend paths.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/dsa/bcm_sf2.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index feb29c4526f7..09f6b3cc1f66 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -400,6 +400,16 @@ static int bcm_sf2_sw_rst(struct bcm_sf2_priv *priv)
return 0;
}
+static void bcm_sf2_intr_disable(struct bcm_sf2_priv *priv)
+{
+ intrl2_0_writel(priv, 0xffffffff, INTRL2_CPU_MASK_SET);
+ intrl2_0_writel(priv, 0xffffffff, INTRL2_CPU_CLEAR);
+ intrl2_0_writel(priv, 0, INTRL2_CPU_MASK_CLEAR);
+ intrl2_1_writel(priv, 0xffffffff, INTRL2_CPU_MASK_SET);
+ intrl2_1_writel(priv, 0xffffffff, INTRL2_CPU_CLEAR);
+ intrl2_1_writel(priv, 0, INTRL2_CPU_MASK_CLEAR);
+}
+
static int bcm_sf2_sw_setup(struct dsa_switch *ds)
{
const char *reg_names[BCM_SF2_REGS_NUM] = BCM_SF2_REGS_NAME;
@@ -440,12 +450,7 @@ static int bcm_sf2_sw_setup(struct dsa_switch *ds)
}
/* Disable all interrupts and request them */
- intrl2_0_writel(priv, 0xffffffff, INTRL2_CPU_MASK_SET);
- intrl2_0_writel(priv, 0xffffffff, INTRL2_CPU_CLEAR);
- intrl2_0_writel(priv, 0, INTRL2_CPU_MASK_CLEAR);
- intrl2_1_writel(priv, 0xffffffff, INTRL2_CPU_MASK_SET);
- intrl2_1_writel(priv, 0xffffffff, INTRL2_CPU_CLEAR);
- intrl2_1_writel(priv, 0, INTRL2_CPU_MASK_CLEAR);
+ bcm_sf2_intr_disable(priv);
ret = request_irq(priv->irq0, bcm_sf2_switch_0_isr, 0,
"switch_0", priv);
@@ -747,12 +752,7 @@ static int bcm_sf2_sw_suspend(struct dsa_switch *ds)
struct bcm_sf2_priv *priv = ds_to_priv(ds);
unsigned int port;
- intrl2_0_writel(priv, 0xffffffff, INTRL2_CPU_MASK_SET);
- intrl2_0_writel(priv, 0xffffffff, INTRL2_CPU_CLEAR);
- intrl2_0_writel(priv, 0, INTRL2_CPU_MASK_CLEAR);
- intrl2_1_writel(priv, 0xffffffff, INTRL2_CPU_MASK_SET);
- intrl2_1_writel(priv, 0xffffffff, INTRL2_CPU_CLEAR);
- intrl2_1_writel(priv, 0, INTRL2_CPU_MASK_CLEAR);
+ bcm_sf2_intr_disable(priv);
/* Disable all ports physically present including the IMP
* port, the other ones have already been disabled during
--
2.1.0
^ permalink raw reply related
* [PATCH net-next 1/2] net: phy: fixed: allow setting no update_link callback
From: Florian Fainelli @ 2015-01-21 0:41 UTC (permalink / raw)
To: netdev; +Cc: davem, Florian Fainelli
In-Reply-To: <1421800920-6281-1-git-send-email-f.fainelli@gmail.com>
fixed_phy_set_link_update() contains an early check against a NULL
callback pointer, which basically prevents us from removing any
previous callback we may have set. The users of the fp->link_update
callback deal with a NULL callback just fine, so we really want to allow
"removing" a link_update callback to avoid dangling callback pointers
during e.g: module removal.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/phy/fixed_phy.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/phy/fixed_phy.c b/drivers/net/phy/fixed_phy.c
index 3ad0e6e16c39..a08a3c78ba97 100644
--- a/drivers/net/phy/fixed_phy.c
+++ b/drivers/net/phy/fixed_phy.c
@@ -168,7 +168,7 @@ int fixed_phy_set_link_update(struct phy_device *phydev,
struct fixed_mdio_bus *fmb = &platform_fmb;
struct fixed_phy *fp;
- if (!link_update || !phydev || !phydev->bus)
+ if (!phydev || !phydev->bus)
return -EINVAL;
list_for_each_entry(fp, &fmb->phys, node) {
--
2.1.0
^ permalink raw reply related
* [PATCH net-next 0/2] net: phy and dsa random fixes/cleanups
From: Florian Fainelli @ 2015-01-21 0:41 UTC (permalink / raw)
To: netdev; +Cc: davem, Florian Fainelli
Hi David,
These two patches were already present as part of my attempt to make
DSA modules work properly, these are the only two "valid" patches at
this point which should not need any further rework.
Thanks!
Florian Fainelli (2):
net: phy: fixed: allow setting no update_link callback
net: dsa: bcm_sf2: factor interrupt disabling in a function
drivers/net/dsa/bcm_sf2.c | 24 ++++++++++++------------
drivers/net/phy/fixed_phy.c | 2 +-
2 files changed, 13 insertions(+), 13 deletions(-)
--
2.1.0
^ permalink raw reply
* Re: [PATCH] net: dsa: set slave MII bus PHY mask
From: Florian Fainelli @ 2015-01-21 0:37 UTC (permalink / raw)
To: Vivien Didelot, netdev; +Cc: David S . Miller, linux-kernel, kernel
In-Reply-To: <1421799212-2028-2-git-send-email-vivien.didelot@savoirfairelinux.com>
On 20/01/15 16:13, Vivien Didelot wrote:
> When registering a mdio bus, Linux assumes than every port has a PHY and tries
> to scan it. If a switch port has no PHY registered, DSA will fail to register
> the slave MII bus. To fix this, set the slave MII bus PHY mask to the switch
> PHYs mask.
>
> As an example, if we use a Marvell MV88E6352 (which is a 7-port switch with no
> registered PHYs for port 5 and port 6), with the following declared names:
>
> static struct dsa_chip_data switch_cdata = {
> [...]
> .port_names[0] = "sw0",
> .port_names[1] = "sw1",
> .port_names[2] = "sw2",
> .port_names[3] = "sw3",
> .port_names[4] = "sw4",
> .port_names[5] = "cpu",
> };
>
> DSA will fail to create the switch instance. With the PHY mask set for the
> slave MII bus, only the PHY for ports 0-4 will be scanned and the instance will
> be successfully created.
>
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Tested-by: Florian Fainelli <f.fainelli@gmail.com>
Acked-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> net/dsa/slave.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/net/dsa/slave.c b/net/dsa/slave.c
> index 515569f..589aafd 100644
> --- a/net/dsa/slave.c
> +++ b/net/dsa/slave.c
> @@ -46,6 +46,7 @@ void dsa_slave_mii_bus_init(struct dsa_switch *ds)
> snprintf(ds->slave_mii_bus->id, MII_BUS_ID_SIZE, "dsa-%d:%.2x",
> ds->index, ds->pd->sw_addr);
> ds->slave_mii_bus->parent = ds->master_dev;
> + ds->slave_mii_bus->phy_mask = ~ds->phys_mii_mask;
> }
>
>
>
--
Florian
^ permalink raw reply
* RE: [E1000-devel] [PATCH 1/2] if_link: Add VF multicast promiscuous mode control
From: Skidmore, Donald C @ 2015-01-21 0:26 UTC (permalink / raw)
To: Hiroshi Shimamoto, Bjørn Mork
Cc: e1000-devel@lists.sourceforge.net, netdev@vger.kernel.org,
Choi, Sy Jong, linux-kernel@vger.kernel.org, Hayato Momma
In-Reply-To: <7F861DC0615E0C47A872E6F3C5FCDDBD05E07B7C@BPXM14GP.gisp.nec.co.jp>
> -----Original Message-----
> From: Hiroshi Shimamoto [mailto:h-shimamoto@ct.jp.nec.com]
> Sent: Tuesday, January 20, 2015 3:40 PM
> To: Bjørn Mork
> Cc: e1000-devel@lists.sourceforge.net; netdev@vger.kernel.org; Choi, Sy
> Jong; linux-kernel@vger.kernel.org; Hayato Momma
> Subject: Re: [E1000-devel] [PATCH 1/2] if_link: Add VF multicast promiscuous
> mode control
>
> > Subject: Re: [PATCH 1/2] if_link: Add VF multicast promiscuous mode
> > control
> >
> > Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com> writes:
> >
> > > From: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
> > >
> > > Add netlink directives and ndo entry to control VF multicast promiscuous
> mode.
> > >
> > > Intel ixgbe and ixgbevf driver can handle only 30 multicast MAC
> > > addresses per VF. It means that we cannot assign over 30 IPv6
> > > addresses to a single VF interface on VM. We want thousands IPv6
> addresses in VM.
> > >
> > > There is capability of multicast promiscuous mode in Intel 82599 chip.
> > > It enables all multicast packets are delivered to the target VF.
> > >
> > > This patch prepares to control that VF multicast promiscuous
> functionality.
> >
> > Adding a new hook for this seems over-complicated to me. And it still
> > doesn't solve the real problems that
> > a) the user has to know about this limit, and
> > b) manually configure the feature
> >
> > Most of us, lacking the ability to imagine such arbitrary hardware
> > limitations, will go through a few hours of frustrating debugging
> > before we figure this one out...
> >
> > Why can't the ixgbevf driver just automatically signal the ixgbe
> > driver to enable multicast promiscuous mode whenever the list grows
> > past the limit?
>
> I had submitted a patch to change ixgbe and ixgbevf driver for this issue.
> https://lkml.org/lkml/2014/11/27/269
>
> The previous patch introduces API between ixgbe and ixgbevf driver to
> enable multicast promiscuous mode, and ixgbevf enables it automatically if
> the number of addresses is over than 30.
I believe the issue is with allowing a VF to automatically enter Promiscuous Multicast without the PF's ok is concern over VM isolation. Of course that isolation, when it comes to multicast, is rather limited anyway given that our multicast filter uses only 12-bit of the address for a match. Still this (or doing it by default) would only open that up considerably more (all multicasts). I assume for your application you're not concerned, but are there other use cases that would worry about such things?
Thanks,
-Don Skidmore <donald.c.skidmore@intel.com>
>
> I got some comment and I would like to clarify the point, but there was no
> answer.
> That's the reason I submitted this patch.
>
> Do you think a patch for the ixgbe/ixgbevf driver is preferred?
>
>
> thanks,
> Hiroshi
>
> >
> > I'd also like to note that this comment in
> > drivers/net/ethernet/intel/ixgbevf/vf.c
> > indicates that the author had some ideas about how more than 30
> > addresses could/should be handled:
> >
> > static s32 ixgbevf_update_mc_addr_list_vf(struct ixgbe_hw *hw,
> > struct net_device *netdev)
> > {
> > struct netdev_hw_addr *ha;
> > u32 msgbuf[IXGBE_VFMAILBOX_SIZE];
> > u16 *vector_list = (u16 *)&msgbuf[1];
> > u32 cnt, i;
> >
> > /* Each entry in the list uses 1 16 bit word. We have 30
> > * 16 bit words available in our HW msg buffer (minus 1 for the
> > * msg type). That's 30 hash values if we pack 'em right. If
> > * there are more than 30 MC addresses to add then punt the
> > * extras for now and then add code to handle more than 30 later.
> > * It would be unusual for a server to request that many multi-cast
> > * addresses except for in large enterprise network environments.
> > */
> >
> >
> >
> > The last 2 lines of that comment are of course totally bogus and
> > pointless and should be deleted in any case... It's obvious that 30
> > multicast addresses is ridiculously low for lots of normal use cases.
> >
> >
> > Bjørn
^ permalink raw reply
* [PATCH] net: dsa: set slave MII bus PHY mask
From: Vivien Didelot @ 2015-01-21 0:13 UTC (permalink / raw)
To: netdev
Cc: Vivien Didelot, David S . Miller, Florian Fainelli, linux-kernel,
kernel
In-Reply-To: <1421799212-2028-1-git-send-email-vivien.didelot@savoirfairelinux.com>
When registering a mdio bus, Linux assumes than every port has a PHY and tries
to scan it. If a switch port has no PHY registered, DSA will fail to register
the slave MII bus. To fix this, set the slave MII bus PHY mask to the switch
PHYs mask.
As an example, if we use a Marvell MV88E6352 (which is a 7-port switch with no
registered PHYs for port 5 and port 6), with the following declared names:
static struct dsa_chip_data switch_cdata = {
[...]
.port_names[0] = "sw0",
.port_names[1] = "sw1",
.port_names[2] = "sw2",
.port_names[3] = "sw3",
.port_names[4] = "sw4",
.port_names[5] = "cpu",
};
DSA will fail to create the switch instance. With the PHY mask set for the
slave MII bus, only the PHY for ports 0-4 will be scanned and the instance will
be successfully created.
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
net/dsa/slave.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 515569f..589aafd 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -46,6 +46,7 @@ void dsa_slave_mii_bus_init(struct dsa_switch *ds)
snprintf(ds->slave_mii_bus->id, MII_BUS_ID_SIZE, "dsa-%d:%.2x",
ds->index, ds->pd->sw_addr);
ds->slave_mii_bus->parent = ds->master_dev;
+ ds->slave_mii_bus->phy_mask = ~ds->phys_mii_mask;
}
--
2.2.2
^ permalink raw reply related
* [PATCH] net: dsa: set parent of hwmon device
From: Vivien Didelot @ 2015-01-21 0:13 UTC (permalink / raw)
To: netdev
Cc: Vivien Didelot, David S . Miller, Guenter Roeck, linux-kernel,
kernel
Set the dsa device as the parent of the hwmon device, in order to link
the hwmon subsystem under the corresponding /sys/devices/platform/dsa.X/
sysfs directory.
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
net/dsa/dsa.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 3731714..363102a 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -347,7 +347,7 @@ dsa_switch_setup(struct dsa_switch_tree *dst, int index,
hname[j] = '\0';
scnprintf(ds->hwmon_name, sizeof(ds->hwmon_name), "%s_dsa%d",
hname, index);
- ds->hwmon_dev = hwmon_device_register_with_groups(NULL,
+ ds->hwmon_dev = hwmon_device_register_with_groups(parent,
ds->hwmon_name, ds, dsa_hwmon_groups);
if (IS_ERR(ds->hwmon_dev))
ds->hwmon_dev = NULL;
--
2.2.2
^ permalink raw reply related
* Re: [PATCH net-next 1/3] skbuff: Add skb_list_linearize()
From: Eric Dumazet @ 2015-01-20 23:51 UTC (permalink / raw)
To: Pravin Shelar; +Cc: David Miller, netdev
In-Reply-To: <CALnjE+pDdvcSdQV83J_DLJwmcM8OsC94xDnH2mSSsxDCNnsztg@mail.gmail.com>
On Tue, 2015-01-20 at 14:46 -0800, Pravin Shelar wrote:
> In current use-case it is never called for skb which is tied to
> socket. But I agree it need to be fixed. I can call skb_orphan() to
> fix it.
I would do nothing at all, like skb_linearize()
^ 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