* Re: [PATCH net-next 0/6] net: phy: marvell: Checkpatch cleanup
From: David Miller @ 2017-05-17 20:28 UTC (permalink / raw)
To: andrew; +Cc: netdev, f.fainelli
In-Reply-To: <1494984364-19496-1-git-send-email-andrew@lunn.ch>
From: Andrew Lunn <andrew@lunn.ch>
Date: Wed, 17 May 2017 03:25:58 +0200
> I will be contributing a few new features to the Marvell PHY driver
> soon. Start by making the code mostly checkpatch clean. There should
> not be any functional changes. Just comments set into the correct
> format, missing blank lines, turn some comparisons around, and
> refactoring to reduce indentation depth.
>
> There is still one camel in the code, but it actually makes sense, so
> leave it in piece.
Series applied, thanks Andrew.
^ permalink raw reply
* Re: [PATCH] net/smc: mark as BROKEN due to remote memory exposure
From: Doug Ledford @ 2017-05-17 20:37 UTC (permalink / raw)
To: David Miller
Cc: Bart.VanAssche, torvalds, hch, netdev, linux-rdma, stable, ubraun
In-Reply-To: <1494962899.3259.141.camel@redhat.com>
On Tue, 2017-05-16 at 15:28 -0400, Doug Ledford wrote:
> I hadn't realized EXPERIMENTAL was gone. Which is too bad, because
> that's entirely appropriate in this case, and would have had the
> desired side effect of keeping it out of any non-cutting edge distros
> and warning people of possible API changes. With EXPERIMENTAL gone,
> the closest thing we have is drivers/staging, since that tends to
> imply
> some of the same consequences. I know you think BROKEN is overly
> harsh, but I'm not sure we should just do nothing. How about we take
> a
> few days to let some of the RDMA people closely review the 143 page
> (egads!) rfc (http://www.rfc-editor.org/info/rfc7609) to see if we
> think it can be fixed to use multiple link layers with the existing
> API
> in place or if it will require something other than AF_SMC. If we
> need
> to break API, then I think we should either fix it ASAP and send that
> fix to the 4.11 stable series (which probably violates the normative
> stable patch size/scope) or if the fix will take longer than this
> kernel cycle, then move it to staging both here and in 4.11 stable,
> and
> fix it there and then move it back. Something like that would
> prevent
> the kind of API flappage we ought not do....
So, I've skimmed the entire RFC, focusing on things were I needed to.
Here's my take on it:
It would have been better with AF_INET/AF_INET6 and an option to enable
SMC than AF_SMC. The first implementation simply assumes AF_INET in
the presence of AF_SMC. When IPv6 support is added, some sort of
guessing logic will have to be put in place to try and determine if an
AF_SMC address is actually AF_INET or AF_INET6 since we won't have a
guaranteed way of telling. Apps can use struct sockaddr_storage as
their normal element to stick the address into, and could rely on the
kernel to interpret it properly based on the AF_INET/AF_INET6
differentiation, and this breaks that. The RFC gives *some* thought to
adding IPv6 in the future, but not a lot. It may be that the answer is
that in the future, IPv6 support is enabled by making the IPv6 API be
AF_INET6 + setsockopt(SMC) or the equivalent. If that's the case, then
I would suggest making the later API specifically call out AF_INET +
setsockopt(SMC) be identical to AF_SMC.
The protocol included a version header in the negotation messages.
This is good as it allows us to almost immediately start work on
version 2 that fixes the shortcomings of version 1 while maintaining
back compatibility.
After reading the RFC, I can see why they only support one link layer:
RoCE. The issue here is that they punted on the hard issue of doing
any sort of work to determine if security restrictions on the TCP
connection should also be applied to the RDMA connection. The RFC
basically says "the RoCE link needs to have the same physical
restrictions (vlan) as the TCP/IP link so that the security limitations
are the same" because they don't do anything to check them essentially.
For v2 of the protocol, and for different link layer support, this is
no longer sufficient, so there will be significant work to determine
the security context of specific TCP connections and then make sure
that they meet the security context of the RDMA links allowed.
Additionally, the same is likely to be true in terms of SELinux
options. The addition of the IB/OPA link layers will throw a bit of a
monkey wrench in things because the SELinux control over those links is
slightly different than a normal TCP/IP SELinux control. For instance,
you might create rules about processes and ports to make sure that the
httpd daemon can only access specific ports on TCP/IP, but on IB/OPA
you would need to create process and P_Key rules as IB/OPA don't have
the same port level semantics, and it's the P_Key on communications
that is enforced network wide, including in the switches, so
controlling what P_Key a process can send/receive on is your best way
to limit what a process can do. Translation from one to the other
might be rather difficult to do in any sort of automated fashion.
There might have to be some additional work to get this to properly
account items for the RDMA control group elements that were recently
taken into the kernel.
Finally, the RDMA subsystem is in the process of switching to
structured option processing similar to how netlink does it. For
version 2 of this protocol, since it will be interacting with the RDMA
core in many ways, will be simpler if it switches the on-wire
negotiation packets to follow the same methods as that will allow reuse
of code that it will have to have for properly dealing with the RDMA
subsystem in the future.
So, I'm fine with it being left as is since you accepted the patch that
makes note of the memory registration insecurity in the Kconfig text.
The fact that this is a versioned protocol means that we can fix the
things we see as being wrong without having to have it all right from
the very start, it can be done incrementally.
--
Doug Ledford <dledford@redhat.com>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD
^ permalink raw reply
* Re: [PATCH net-next 3/3] dsa: mv88e6xxx: Enable/Disable SERDES on port enable/disable
From: Vivien Didelot @ 2017-05-17 20:44 UTC (permalink / raw)
To: Andrew Lunn, David Miller; +Cc: netdev, nikita.yoush, Andrew Lunn
In-Reply-To: <1495051512-13554-4-git-send-email-andrew@lunn.ch>
Hi Andrew,
Good patchset overall, some comments below.
Andrew Lunn <andrew@lunn.ch> writes:
> @@ -2168,7 +2165,44 @@ static int mv88e6xxx_setup_port(struct mv88e6xxx_chip *chip, int port)
> /* Default VLAN ID and priority: don't set a default VLAN
> * ID, and set the default packet priority to zero.
> */
> - return mv88e6xxx_port_write(chip, port, PORT_DEFAULT_VLAN, 0x0000);
> + err = mv88e6xxx_port_write(chip, port, PORT_DEFAULT_VLAN, 0x0000);
> + if (err)
> + return err;
> +
> + /* Enable the SERDES interface for DSA and CPU ports. Normal
> + * ports SERDES are enabled when the port is enabled, thus
> + * saving a bit of power.
> + */
> + if ((dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port)) &&
> + chip->info->ops->serdes_power)
> + err = chip->info->ops->serdes_power(chip, port, true);
Please provide a wrapper like this in chip.c in patch 1/3:
static int mv88e6xxx_serdes_power()
{
if (chip->info->ops->serdes_power)
return chip->info->ops->serdes_power();
return 0;
}
So that we don't check chip->info->ops that many times.
> +
> + return err;
> +}
> +
> +static int mv88e6xxx_port_enable(struct dsa_switch *ds, int port,
> + struct phy_device *phydev)
> +{
> + struct mv88e6xxx_chip *chip = ds->priv;
> + int err = 0;
> +
> + mutex_lock(&chip->reg_lock);
> + if (chip->info->ops->serdes_power)
> + err = chip->info->ops->serdes_power(chip, port, true);
> + mutex_unlock(&chip->reg_lock);
> +
> + return err;
> +}
> +
> +static void mv88e6xxx_port_disable(struct dsa_switch *ds, int port,
> + struct phy_device *phydev)
> +{
> + struct mv88e6xxx_chip *chip = ds->priv;
> +
> + mutex_lock(&chip->reg_lock);
> + if (chip->info->ops->serdes_power)
> + chip->info->ops->serdes_power(chip, port, false);
> + mutex_unlock(&chip->reg_lock);
> }
Also please split this in 2 patchs, one which setups serdes with the
port, one which implements port_enable/disable DSA ops.
Thanks,
Vivien
^ permalink raw reply
* [net-intel-i40e] question about assignment overwrite
From: Gustavo A. R. Silva @ 2017-05-17 20:48 UTC (permalink / raw)
To: Jeff Kirsher; +Cc: intel-wired-lan, netdev, linux-kernel
Hello everybody,
While looking into Coverity ID 1408956 I ran into the following piece
of code at drivers/net/ethernet/intel/i40e/i40e_main.c:8807:
8807 if (pf->hw.mac.type == I40E_MAC_X722) {
8808 pf->flags |= I40E_FLAG_RSS_AQ_CAPABLE
8809 | I40E_FLAG_128_QP_RSS_CAPABLE
8810 | I40E_FLAG_HW_ATR_EVICT_CAPABLE
8811 | I40E_FLAG_OUTER_UDP_CSUM_CAPABLE
8812 | I40E_FLAG_WB_ON_ITR_CAPABLE
8813 | I40E_FLAG_MULTIPLE_TCP_UDP_RSS_PCTYPE
8814 | I40E_FLAG_NO_PCI_LINK_CHECK
8815 | I40E_FLAG_USE_SET_LLDP_MIB
8816 | I40E_FLAG_GENEVE_OFFLOAD_CAPABLE
8817 | I40E_FLAG_PTP_L4_CAPABLE
8818 | I40E_FLAG_WOL_MC_MAGIC_PKT_WAKE;
8819 } else if ((pf->hw.aq.api_maj_ver > 1) ||
8820 ((pf->hw.aq.api_maj_ver == 1) &&
8821 (pf->hw.aq.api_min_ver > 4))) {
8822 /* Supported in FW API version higher than 1.4 */
8823 pf->flags |= I40E_FLAG_GENEVE_OFFLOAD_CAPABLE;
8824 pf->flags = I40E_FLAG_HW_ATR_EVICT_CAPABLE;
8825 } else {
8826 pf->flags = I40E_FLAG_HW_ATR_EVICT_CAPABLE;
8827 }
The issue here is that the assignment at line 8823 is overwritten by
the code at line 8824.
I'm suspicious that line 8824 should be remove and a patch like the
following can be applied:
index d5c9c9e..48ffa73 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -8821,7 +8821,6 @@ static int i40e_sw_init(struct i40e_pf *pf)
(pf->hw.aq.api_min_ver > 4))) {
/* Supported in FW API version higher than 1.4 */
pf->flags |= I40E_FLAG_GENEVE_OFFLOAD_CAPABLE;
- pf->flags = I40E_FLAG_HW_ATR_EVICT_CAPABLE;
} else {
pf->flags = I40E_FLAG_HW_ATR_EVICT_CAPABLE;
}
What do you think?
Thanks!
--
Gustavo A. R. Silva
^ permalink raw reply related
* Re: [PATCH net-next 2/3] net: dsa: mv88e6xxx: mv88e6390X SERDES support
From: Vivien Didelot @ 2017-05-17 20:49 UTC (permalink / raw)
To: Andrew Lunn, David Miller; +Cc: netdev, nikita.yoush, Andrew Lunn
In-Reply-To: <1495051512-13554-3-git-send-email-andrew@lunn.ch>
Hi Andrew,
Andrew Lunn <andrew@lunn.ch> writes:
> -static int mv88e6xxx_phy_read(struct mv88e6xxx_chip *chip, int phy,
> - int reg, u16 *val)
> +int mv88e6xxx_phy_read(struct mv88e6xxx_chip *chip, int phy,
> + int reg, u16 *val)
> {
> int addr = phy; /* PHY devices addresses start at 0x0 */
> struct mii_bus *bus;
> @@ -265,8 +265,8 @@ static int mv88e6xxx_phy_read(struct mv88e6xxx_chip *chip, int phy,
> return chip->info->ops->phy_read(chip, bus, addr, reg, val);
> }
>
> -static int mv88e6xxx_phy_write(struct mv88e6xxx_chip *chip, int phy,
> - int reg, u16 val)
> +int mv88e6xxx_phy_write(struct mv88e6xxx_chip *chip, int phy,
> + int reg, u16 val)
Please add a very first patch which expose the mv88e6xxx_phy_ (and page_
of patch 1/3) PHY related code in new phy.[ch] files. These are the
missing specific files for the PHY Registers sets ;-)
> + if (chip->info->ops->serdes_power)
> + return chip->info->ops->serdes_power(chip, port, true);
> +
> + return 0;
> }
>
> static int mv88e6xxx_setup_message_port(struct mv88e6xxx_chip *chip, int port)
> @@ -2068,15 +2071,6 @@ static int mv88e6xxx_setup_port(struct mv88e6xxx_chip *chip, int port)
> if (err)
> return err;
>
> - /* If this port is connected to a SerDes, make sure the SerDes is
> - * powered up.
> - */
> - if (chip->info->ops->serdes_power) {
> - err = chip->info->ops->serdes_power(chip, port, true);
> - if (err)
> - return err;
> - }
> -
All 3 patches moves this call around. Can we place it right once, the
first time a mv88e6xxx_serdes_power() helper is added?
Thank you,
Vivien
^ permalink raw reply
* Re: [PATCH net-next V5 0/9] vhost_net rx batch dequeuing
From: Michael S. Tsirkin @ 2017-05-17 20:59 UTC (permalink / raw)
To: Jason Wang; +Cc: netdev, linux-kernel
In-Reply-To: <1494994485-12994-1-git-send-email-jasowang@redhat.com>
On Wed, May 17, 2017 at 12:14:36PM +0800, Jason Wang wrote:
> This series tries to implement rx batching for vhost-net. This is done
> by batching the dequeuing from skb_array which was exported by
> underlayer socket and pass the sbk back through msg_control to finish
> userspace copying. This is also the requirement for more batching
> implemention on rx path.
>
> Tests shows at most 7.56% improvment bon rx pps on top of batch
> zeroing and no obvious changes for TCP_STREAM/TCP_RR result.
>
> Please review.
>
> Thanks
A surprisingly large gain for such as simple change. It would be nice
to understand better why this helps - in particular, does the optimal
batch size change if ring is bigger or smaller? But let's merge it
meanwhile.
Series:
Acked-by: Michael S. Tsirkin <mst@redhat.com>
> Changes from V4:
> - drop batch zeroing patch
> - renew the performance numbers
> - move skb pointer array out of vhost_net structure
>
> Changes from V3:
> - add batch zeroing patch to fix the build warnings
>
> Changes from V2:
> - rebase to net-next HEAD
> - use unconsume helpers to put skb back on releasing
> - introduce and use vhost_net internal buffer helpers
> - renew performance numbers on top of batch zeroing
>
> Changes from V1:
> - switch to use for() in __ptr_ring_consume_batched()
> - rename peek_head_len_batched() to fetch_skbs()
> - use skb_array_consume_batched() instead of
> skb_array_consume_batched_bh() since no consumer run in bh
> - drop the lockless peeking patch since skb_array could be resized, so
> it's not safe to call lockless one
>
> Jason Wang (8):
> skb_array: introduce skb_array_unconsume
> ptr_ring: introduce batch dequeuing
> skb_array: introduce batch dequeuing
> tun: export skb_array
> tap: export skb_array
> tun: support receiving skb through msg_control
> tap: support receiving skb from msg_control
> vhost_net: try batch dequing from skb array
>
> Michael S. Tsirkin (1):
> ptr_ring: add ptr_ring_unconsume
>
> drivers/net/tap.c | 25 +++++++--
> drivers/net/tun.c | 31 ++++++++---
> drivers/vhost/net.c | 128 +++++++++++++++++++++++++++++++++++++++++++---
> include/linux/if_tap.h | 5 ++
> include/linux/if_tun.h | 5 ++
> include/linux/ptr_ring.h | 120 +++++++++++++++++++++++++++++++++++++++++++
> include/linux/skb_array.h | 31 +++++++++++
> 7 files changed, 327 insertions(+), 18 deletions(-)
>
> --
> 2.7.4
^ permalink raw reply
* Re: [PATCH net-next 1/3] net: dsa: mv88e6xxx: Refactor mv88e6352 SERDES code into an op
From: Vivien Didelot @ 2017-05-17 21:01 UTC (permalink / raw)
To: Andrew Lunn, David Miller; +Cc: netdev, nikita.yoush, Andrew Lunn
In-Reply-To: <1495051512-13554-2-git-send-email-andrew@lunn.ch>
Hi Andrew,
Andrew Lunn <andrew@lunn.ch> writes:
> static int mv88e6xxx_phy_page_get(struct mv88e6xxx_chip *chip, int phy, u8 page)
> {
> - if (!mv88e6xxx_has(chip, MV88E6XXX_FLAG_PHY_PAGE))
> - return -EOPNOTSUPP;
> -
> return mv88e6xxx_phy_write(chip, phy, PHY_PAGE, page);
> }
>
> @@ -300,8 +298,8 @@ static void mv88e6xxx_phy_page_put(struct mv88e6xxx_chip *chip, int phy)
> }
> }
>
> -static int mv88e6xxx_phy_page_read(struct mv88e6xxx_chip *chip, int phy,
> - u8 page, int reg, u16 *val)
> +int mv88e6xxx_phy_page_read(struct mv88e6xxx_chip *chip, int phy,
> + u8 page, int reg, u16 *val)
> {
> int err;
>
> @@ -318,8 +316,8 @@ static int mv88e6xxx_phy_page_read(struct mv88e6xxx_chip *chip, int phy,
> return err;
> }
>
> -static int mv88e6xxx_phy_page_write(struct mv88e6xxx_chip *chip, int phy,
> - u8 page, int reg, u16 val)
> +int mv88e6xxx_phy_page_write(struct mv88e6xxx_chip *chip, int phy,
> + u8 page, int reg, u16 val)
Hum, this patch does a bit too much... Please at least add a first one
providing phy.[ch] for the PHY Registers related functions, then add the
serdes_power operation.
> +#define MV88E6XXX_FLAG_G1_ATU_FID BIT_ULL(MV88E6XXX_CAP_G1_ATU_FID)
This seems to be added back by mistake.
> @@ -889,6 +868,9 @@ struct mv88e6xxx_ops {
> struct mv88e6xxx_vtu_entry *entry);
> int (*vtu_loadpurge)(struct mv88e6xxx_chip *chip,
> struct mv88e6xxx_vtu_entry *entry);
> +
> + /* Power on/off a SERDES interface */
> + int (*serdes_power)(struct mv88e6xxx_chip *chip, int port, bool on);
> };
While the get_* and set_* ops are an exception, we might want to sort
that at least before the vtu_* ops. But that is a minor comment.
> +#define MV88E6352_ADDR_SERDES 0x0f
> +#define MV88E6352_SERDES_PAGE_FIBER 0x01
Why not moving this in the serdes.h header with the others?
Thank you,
Vivien
^ permalink raw reply
* [PATCH net-next 0/6] net: bridge: factorize MDB new and del functions
From: Vivien Didelot @ 2017-05-17 21:27 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel, kernel, David S. Miller, Vivien Didelot
The br_mdb.c file implements both br_mdb_add and br_mdb_del functions, to
handle respectively the RTM_NEWMDB and RTM_DELMDB message types.
But these two functions are really similar. This patch series factorizes
them into a single br_mdb_do rtnl_doit_func function to reduce
boilerplate and simplify the MDB handling code.
Vivien Didelot (6):
net: bridge: pass net_bridge_port to __br_mdb_add
net: bridge: check multicast bridge only once
net: bridge: break if __br_mdb_del fails
net: bridge: add __br_mdb_do
net: bridge: get msgtype from nlmsghdr in mdb ops
net: bridge: add br_mdb_do
net/bridge/br_mdb.c | 107 +++++++++++++++-------------------------------------
1 file changed, 31 insertions(+), 76 deletions(-)
--
2.13.0
^ permalink raw reply
* [PATCH net-next 1/6] net: bridge: pass net_bridge_port to __br_mdb_add
From: Vivien Didelot @ 2017-05-17 21:27 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, kernel, David S. Miller, Vivien Didelot,
Stephen Hemminger, moderated list:ETHERNET BRIDGE
In-Reply-To: <20170517212709.6473-1-vivien.didelot@savoirfairelinux.com>
The __br_mdb_add function uses exactly the same mechanism as its caller
br_mdb_add to retrieve the net_bridge_port pointer from br_mdb_entry.
Remove it and pass directly the net_bridge_port pointer to __br_mdb_add.
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
net/bridge/br_mdb.c | 18 ++++--------------
1 file changed, 4 insertions(+), 14 deletions(-)
diff --git a/net/bridge/br_mdb.c b/net/bridge/br_mdb.c
index b0845480a3ae..bef0331f46a4 100644
--- a/net/bridge/br_mdb.c
+++ b/net/bridge/br_mdb.c
@@ -542,25 +542,15 @@ static int br_mdb_add_group(struct net_bridge *br, struct net_bridge_port *port,
return 0;
}
-static int __br_mdb_add(struct net *net, struct net_bridge *br,
- struct br_mdb_entry *entry)
+static int __br_mdb_add(struct net_bridge_port *p, struct br_mdb_entry *entry)
{
+ struct net_bridge *br = p->br;
struct br_ip ip;
- struct net_device *dev;
- struct net_bridge_port *p;
int ret;
if (!netif_running(br->dev) || br->multicast_disabled)
return -EINVAL;
- dev = __dev_get_by_index(net, entry->ifindex);
- if (!dev)
- return -ENODEV;
-
- p = br_port_get_rtnl(dev);
- if (!p || p->br != br || p->state == BR_STATE_DISABLED)
- return -EINVAL;
-
__mdb_entry_to_br_ip(entry, &ip);
spin_lock_bh(&br->multicast_lock);
@@ -602,13 +592,13 @@ static int br_mdb_add(struct sk_buff *skb, struct nlmsghdr *nlh,
if (br_vlan_enabled(br) && vg && entry->vid == 0) {
list_for_each_entry(v, &vg->vlan_list, vlist) {
entry->vid = v->vid;
- err = __br_mdb_add(net, br, entry);
+ err = __br_mdb_add(p, entry);
if (err)
break;
__br_mdb_notify(dev, p, entry, RTM_NEWMDB);
}
} else {
- err = __br_mdb_add(net, br, entry);
+ err = __br_mdb_add(p, entry);
if (!err)
__br_mdb_notify(dev, p, entry, RTM_NEWMDB);
}
--
2.13.0
^ permalink raw reply related
* [PATCH net-next 2/6] net: bridge: check multicast bridge only once
From: Vivien Didelot @ 2017-05-17 21:27 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, kernel, David S. Miller, Vivien Didelot,
Stephen Hemminger, moderated list:ETHERNET BRIDGE
In-Reply-To: <20170517212709.6473-1-vivien.didelot@savoirfairelinux.com>
__br_mdb_add and __br_mdb_del both check that the bridge is up and
running and that multicast is enabled on it before doing any operation.
Since they can be called multiple times by br_mdb_add and br_mdb_del,
move the check in their caller functions so that it is done just once.
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
net/bridge/br_mdb.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/net/bridge/br_mdb.c b/net/bridge/br_mdb.c
index bef0331f46a4..d20a01622b20 100644
--- a/net/bridge/br_mdb.c
+++ b/net/bridge/br_mdb.c
@@ -548,9 +548,6 @@ static int __br_mdb_add(struct net_bridge_port *p, struct br_mdb_entry *entry)
struct br_ip ip;
int ret;
- if (!netif_running(br->dev) || br->multicast_disabled)
- return -EINVAL;
-
__mdb_entry_to_br_ip(entry, &ip);
spin_lock_bh(&br->multicast_lock);
@@ -577,6 +574,9 @@ static int br_mdb_add(struct sk_buff *skb, struct nlmsghdr *nlh,
br = netdev_priv(dev);
+ if (!netif_running(br->dev) || br->multicast_disabled)
+ return -EINVAL;
+
/* If vlan filtering is enabled and VLAN is not specified
* install mdb entry on all vlans configured on the port.
*/
@@ -615,9 +615,6 @@ static int __br_mdb_del(struct net_bridge *br, struct br_mdb_entry *entry)
struct br_ip ip;
int err = -EINVAL;
- if (!netif_running(br->dev) || br->multicast_disabled)
- return -EINVAL;
-
__mdb_entry_to_br_ip(entry, &ip);
spin_lock_bh(&br->multicast_lock);
@@ -672,6 +669,9 @@ static int br_mdb_del(struct sk_buff *skb, struct nlmsghdr *nlh,
br = netdev_priv(dev);
+ if (!netif_running(br->dev) || br->multicast_disabled)
+ return -EINVAL;
+
/* If vlan filtering is enabled and VLAN is not specified
* delete mdb entry on all vlans configured on the port.
*/
--
2.13.0
^ permalink raw reply related
* [PATCH net-next 3/6] net: bridge: break if __br_mdb_del fails
From: Vivien Didelot @ 2017-05-17 21:27 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, kernel, David S. Miller, Vivien Didelot,
Stephen Hemminger, moderated list:ETHERNET BRIDGE
In-Reply-To: <20170517212709.6473-1-vivien.didelot@savoirfairelinux.com>
Be symmetric with br_mdb_add and break if __br_mdb_del returns an error.
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
net/bridge/br_mdb.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/net/bridge/br_mdb.c b/net/bridge/br_mdb.c
index d20a01622b20..24eeeefb4179 100644
--- a/net/bridge/br_mdb.c
+++ b/net/bridge/br_mdb.c
@@ -688,8 +688,9 @@ static int br_mdb_del(struct sk_buff *skb, struct nlmsghdr *nlh,
list_for_each_entry(v, &vg->vlan_list, vlist) {
entry->vid = v->vid;
err = __br_mdb_del(br, entry);
- if (!err)
- __br_mdb_notify(dev, p, entry, RTM_DELMDB);
+ if (err)
+ break;
+ __br_mdb_notify(dev, p, entry, RTM_DELMDB);
}
} else {
err = __br_mdb_del(br, entry);
--
2.13.0
^ permalink raw reply related
* [PATCH net-next 4/6] net: bridge: add __br_mdb_do
From: Vivien Didelot @ 2017-05-17 21:27 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, kernel, David S. Miller, Vivien Didelot,
Stephen Hemminger, moderated list:ETHERNET BRIDGE
In-Reply-To: <20170517212709.6473-1-vivien.didelot@savoirfairelinux.com>
br_mdb_add and br_mdb_del call respectively __br_mdb_add and
__br_mdb_del and notify the message type on success.
Factorize this in a new __br_mdb_do function which add or delete a
br_mdb_entry depending on the message type, then notify it.
Note that the dev argument is p->br->dev, so no need to pass it twice.
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
net/bridge/br_mdb.c | 33 +++++++++++++++++++++++----------
1 file changed, 23 insertions(+), 10 deletions(-)
diff --git a/net/bridge/br_mdb.c b/net/bridge/br_mdb.c
index 24eeeefb4179..a72d5e6f339f 100644
--- a/net/bridge/br_mdb.c
+++ b/net/bridge/br_mdb.c
@@ -556,6 +556,9 @@ static int __br_mdb_add(struct net_bridge_port *p, struct br_mdb_entry *entry)
return ret;
}
+static int __br_mdb_do(struct net_bridge_port *p, struct br_mdb_entry *entry,
+ int msgtype);
+
static int br_mdb_add(struct sk_buff *skb, struct nlmsghdr *nlh,
struct netlink_ext_ack *extack)
{
@@ -592,15 +595,12 @@ static int br_mdb_add(struct sk_buff *skb, struct nlmsghdr *nlh,
if (br_vlan_enabled(br) && vg && entry->vid == 0) {
list_for_each_entry(v, &vg->vlan_list, vlist) {
entry->vid = v->vid;
- err = __br_mdb_add(p, entry);
+ err = __br_mdb_do(p, entry, RTM_NEWMDB);
if (err)
break;
- __br_mdb_notify(dev, p, entry, RTM_NEWMDB);
}
} else {
- err = __br_mdb_add(p, entry);
- if (!err)
- __br_mdb_notify(dev, p, entry, RTM_NEWMDB);
+ err = __br_mdb_do(p, entry, RTM_NEWMDB);
}
return err;
@@ -651,6 +651,22 @@ static int __br_mdb_del(struct net_bridge *br, struct br_mdb_entry *entry)
return err;
}
+static int __br_mdb_do(struct net_bridge_port *p, struct br_mdb_entry *entry,
+ int msgtype)
+{
+ int err = -EINVAL;
+
+ if (msgtype == RTM_NEWMDB)
+ err = __br_mdb_add(p, entry);
+ else if (msgtype == RTM_DELMDB)
+ err = __br_mdb_del(p->br, entry);
+
+ if (!err)
+ __br_mdb_notify(p->br->dev, p, entry, msgtype);
+
+ return err;
+}
+
static int br_mdb_del(struct sk_buff *skb, struct nlmsghdr *nlh,
struct netlink_ext_ack *extack)
{
@@ -687,15 +703,12 @@ static int br_mdb_del(struct sk_buff *skb, struct nlmsghdr *nlh,
if (br_vlan_enabled(br) && vg && entry->vid == 0) {
list_for_each_entry(v, &vg->vlan_list, vlist) {
entry->vid = v->vid;
- err = __br_mdb_del(br, entry);
+ err = __br_mdb_do(p, entry, RTM_DELMDB);
if (err)
break;
- __br_mdb_notify(dev, p, entry, RTM_DELMDB);
}
} else {
- err = __br_mdb_del(br, entry);
- if (!err)
- __br_mdb_notify(dev, p, entry, RTM_DELMDB);
+ err = __br_mdb_do(p, entry, RTM_DELMDB);
}
return err;
--
2.13.0
^ permalink raw reply related
* [PATCH net-next 5/6] net: bridge: get msgtype from nlmsghdr in mdb ops
From: Vivien Didelot @ 2017-05-17 21:27 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, kernel, David S. Miller, Vivien Didelot,
Stephen Hemminger, moderated list:ETHERNET BRIDGE
In-Reply-To: <20170517212709.6473-1-vivien.didelot@savoirfairelinux.com>
Retrieve the message type from the nlmsghdr structure instead of
hardcoding it in both br_mdb_add and br_mdb_del.
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
net/bridge/br_mdb.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/net/bridge/br_mdb.c b/net/bridge/br_mdb.c
index a72d5e6f339f..d280b20587cb 100644
--- a/net/bridge/br_mdb.c
+++ b/net/bridge/br_mdb.c
@@ -569,6 +569,7 @@ static int br_mdb_add(struct sk_buff *skb, struct nlmsghdr *nlh,
struct net_bridge_port *p;
struct net_bridge_vlan *v;
struct net_bridge *br;
+ int msgtype = nlh->nlmsg_type;
int err;
err = br_mdb_parse(skb, nlh, &dev, &entry);
@@ -595,12 +596,12 @@ static int br_mdb_add(struct sk_buff *skb, struct nlmsghdr *nlh,
if (br_vlan_enabled(br) && vg && entry->vid == 0) {
list_for_each_entry(v, &vg->vlan_list, vlist) {
entry->vid = v->vid;
- err = __br_mdb_do(p, entry, RTM_NEWMDB);
+ err = __br_mdb_do(p, entry, msgtype);
if (err)
break;
}
} else {
- err = __br_mdb_do(p, entry, RTM_NEWMDB);
+ err = __br_mdb_do(p, entry, msgtype);
}
return err;
@@ -677,6 +678,7 @@ static int br_mdb_del(struct sk_buff *skb, struct nlmsghdr *nlh,
struct net_bridge_port *p;
struct net_bridge_vlan *v;
struct net_bridge *br;
+ int msgtype = nlh->nlmsg_type;
int err;
err = br_mdb_parse(skb, nlh, &dev, &entry);
@@ -703,12 +705,12 @@ static int br_mdb_del(struct sk_buff *skb, struct nlmsghdr *nlh,
if (br_vlan_enabled(br) && vg && entry->vid == 0) {
list_for_each_entry(v, &vg->vlan_list, vlist) {
entry->vid = v->vid;
- err = __br_mdb_do(p, entry, RTM_DELMDB);
+ err = __br_mdb_do(p, entry, msgtype);
if (err)
break;
}
} else {
- err = __br_mdb_do(p, entry, RTM_DELMDB);
+ err = __br_mdb_do(p, entry, msgtype);
}
return err;
--
2.13.0
^ permalink raw reply related
* [PATCH net-next 6/6] net: bridge: add br_mdb_do
From: Vivien Didelot @ 2017-05-17 21:27 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, kernel, David S. Miller, Vivien Didelot,
Stephen Hemminger, moderated list:ETHERNET BRIDGE
In-Reply-To: <20170517212709.6473-1-vivien.didelot@savoirfairelinux.com>
Now that the two functions br_mdb_add and br_mdb_del are identical, keep
a single function named br_mdb_do and register it for both RTM_NEWMDB
and RTM_DELMDB message types.
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
net/bridge/br_mdb.c | 61 +++++------------------------------------------------
1 file changed, 5 insertions(+), 56 deletions(-)
diff --git a/net/bridge/br_mdb.c b/net/bridge/br_mdb.c
index d280b20587cb..1a1da25f3727 100644
--- a/net/bridge/br_mdb.c
+++ b/net/bridge/br_mdb.c
@@ -556,57 +556,6 @@ static int __br_mdb_add(struct net_bridge_port *p, struct br_mdb_entry *entry)
return ret;
}
-static int __br_mdb_do(struct net_bridge_port *p, struct br_mdb_entry *entry,
- int msgtype);
-
-static int br_mdb_add(struct sk_buff *skb, struct nlmsghdr *nlh,
- struct netlink_ext_ack *extack)
-{
- struct net *net = sock_net(skb->sk);
- struct net_bridge_vlan_group *vg;
- struct net_device *dev, *pdev;
- struct br_mdb_entry *entry;
- struct net_bridge_port *p;
- struct net_bridge_vlan *v;
- struct net_bridge *br;
- int msgtype = nlh->nlmsg_type;
- int err;
-
- err = br_mdb_parse(skb, nlh, &dev, &entry);
- if (err < 0)
- return err;
-
- br = netdev_priv(dev);
-
- if (!netif_running(br->dev) || br->multicast_disabled)
- return -EINVAL;
-
- /* If vlan filtering is enabled and VLAN is not specified
- * install mdb entry on all vlans configured on the port.
- */
- pdev = __dev_get_by_index(net, entry->ifindex);
- if (!pdev)
- return -ENODEV;
-
- p = br_port_get_rtnl(pdev);
- if (!p || p->br != br || p->state == BR_STATE_DISABLED)
- return -EINVAL;
-
- vg = nbp_vlan_group(p);
- if (br_vlan_enabled(br) && vg && entry->vid == 0) {
- list_for_each_entry(v, &vg->vlan_list, vlist) {
- entry->vid = v->vid;
- err = __br_mdb_do(p, entry, msgtype);
- if (err)
- break;
- }
- } else {
- err = __br_mdb_do(p, entry, msgtype);
- }
-
- return err;
-}
-
static int __br_mdb_del(struct net_bridge *br, struct br_mdb_entry *entry)
{
struct net_bridge_mdb_htable *mdb;
@@ -668,8 +617,8 @@ static int __br_mdb_do(struct net_bridge_port *p, struct br_mdb_entry *entry,
return err;
}
-static int br_mdb_del(struct sk_buff *skb, struct nlmsghdr *nlh,
- struct netlink_ext_ack *extack)
+static int br_mdb_do(struct sk_buff *skb, struct nlmsghdr *nlh,
+ struct netlink_ext_ack *extack)
{
struct net *net = sock_net(skb->sk);
struct net_bridge_vlan_group *vg;
@@ -691,7 +640,7 @@ static int br_mdb_del(struct sk_buff *skb, struct nlmsghdr *nlh,
return -EINVAL;
/* If vlan filtering is enabled and VLAN is not specified
- * delete mdb entry on all vlans configured on the port.
+ * add or delete mdb entry on all vlans configured on the port.
*/
pdev = __dev_get_by_index(net, entry->ifindex);
if (!pdev)
@@ -719,8 +668,8 @@ static int br_mdb_del(struct sk_buff *skb, struct nlmsghdr *nlh,
void br_mdb_init(void)
{
rtnl_register(PF_BRIDGE, RTM_GETMDB, NULL, br_mdb_dump, NULL);
- rtnl_register(PF_BRIDGE, RTM_NEWMDB, br_mdb_add, NULL, NULL);
- rtnl_register(PF_BRIDGE, RTM_DELMDB, br_mdb_del, NULL, NULL);
+ rtnl_register(PF_BRIDGE, RTM_NEWMDB, br_mdb_do, NULL, NULL);
+ rtnl_register(PF_BRIDGE, RTM_DELMDB, br_mdb_do, NULL, NULL);
}
void br_mdb_uninit(void)
--
2.13.0
^ permalink raw reply related
* Re: [PATCH net-next] drivers: net: xgene: Check all RGMII phy mode variants
From: Iyappan Subramanian @ 2017-05-17 21:29 UTC (permalink / raw)
To: Andrew Lunn
Cc: Quan Nguyen, Florian Fainelli, netdev, patches, David Miller,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20170517202633.GA14413@lunn.ch>
On Wed, May 17, 2017 at 1:26 PM, Andrew Lunn <andrew@lunn.ch> wrote:
>> +bool is_xgene_enet_phy_mode_rgmii(struct net_device *ndev)
>> +{
>> + struct xgene_enet_pdata *pdata = netdev_priv(ndev);
>> + int phy_mode = pdata->phy_mode;
>> + bool ret;
>> +
>> + ret = phy_mode == PHY_INTERFACE_MODE_RGMII ||
>> + phy_mode == PHY_INTERFACE_MODE_RGMII_ID ||
>> + phy_mode == PHY_INTERFACE_MODE_RGMII_RXID ||
>> + phy_mode == PHY_INTERFACE_MODE_RGMII_TXID;
>> +
>> + return ret;
>> +}
>
> include/linux/phy.h:
>
> /**
> * phy_interface_is_rgmii - Convenience function for testing if a PHY interface
> * is RGMII (all variants)
> * @phydev: the phy_device struct
> */
> static inline bool phy_interface_is_rgmii(struct phy_device *phydev)
> {
> return phydev->interface >= PHY_INTERFACE_MODE_RGMII &&
> phydev->interface <= PHY_INTERFACE_MODE_RGMII_TXID;
> };
Thanks. I'll use this helper function.
>
> Andrew
^ permalink raw reply
* [PATCH 0/2] arp: always override existing neigh entries with gratuitous ARP
From: Ihar Hrachyshka @ 2017-05-17 21:33 UTC (permalink / raw)
To: davem, ja; +Cc: Ihar Hrachyshka, netdev
Good day.
This patchset is spurred by discussion started at
https://patchwork.ozlabs.org/patch/760372/ where we figured that there is no
real reason for enforcing override by gratuitous ARP packets only when
arp_accept is 1. Same should happen when it's 0 (the default value).
The first patch in the series moves is_garp code into a separate function
preparing the code base for the 2nd patch that actually implements the needed
change.
Ihar Hrachyshka (2):
arp: decompose is_garp logic into a separate function
arp: always override existing neigh entries with gratuitous ARP
net/ipv4/arp.c | 47 +++++++++++++++++++++++++++++------------------
1 file changed, 29 insertions(+), 18 deletions(-)
--
2.9.3
^ permalink raw reply
* Re: [PATCH net-next] drivers: net: xgene: Check all RGMII phy mode variants
From: Florian Fainelli @ 2017-05-17 21:33 UTC (permalink / raw)
To: Iyappan Subramanian, Andrew Lunn
Cc: netdev, patches, David Miller,
linux-arm-kernel@lists.infradead.org, Quan Nguyen
In-Reply-To: <CAKh23FkpYd0vC-AjqQu9ZStsRNp=kFh90eRfTU5WCC6Ln7E2AA@mail.gmail.com>
On 05/17/2017 02:29 PM, Iyappan Subramanian wrote:
> On Wed, May 17, 2017 at 1:26 PM, Andrew Lunn <andrew@lunn.ch> wrote:
>>> +bool is_xgene_enet_phy_mode_rgmii(struct net_device *ndev)
>>> +{
>>> + struct xgene_enet_pdata *pdata = netdev_priv(ndev);
>>> + int phy_mode = pdata->phy_mode;
>>> + bool ret;
>>> +
>>> + ret = phy_mode == PHY_INTERFACE_MODE_RGMII ||
>>> + phy_mode == PHY_INTERFACE_MODE_RGMII_ID ||
>>> + phy_mode == PHY_INTERFACE_MODE_RGMII_RXID ||
>>> + phy_mode == PHY_INTERFACE_MODE_RGMII_TXID;
>>> +
>>> + return ret;
>>> +}
>>
>> include/linux/phy.h:
>>
>> /**
>> * phy_interface_is_rgmii - Convenience function for testing if a PHY interface
>> * is RGMII (all variants)
>> * @phydev: the phy_device struct
>> */
>> static inline bool phy_interface_is_rgmii(struct phy_device *phydev)
>> {
>> return phydev->interface >= PHY_INTERFACE_MODE_RGMII &&
>> phydev->interface <= PHY_INTERFACE_MODE_RGMII_TXID;
>> };
>
> Thanks. I'll use this helper function.
If you can use it, that's great, the reason why I did not recommend
using it before was because it takes a phydev reference and your code
clearly could have run before connecting to the PHY device.
Alternatively, we could introduce a helper function that just checks a
phy_interface_t type, something like:
static inline bool phy_interface_is_rgmii(phy_interface_t mode)
{
...
}
and introduce:
static inline bool phydev_interface_is_rgmii(struct phy_device *phydev)
{
return phy_interface_is_rgmii(phydev->interface);
}
which would use this helper function internally. Or just drop the second
helper, and always pass phydev->interface where needed.
--
Florian
^ permalink raw reply
* [PATCH 1/2] arp: decompose is_garp logic into a separate function
From: Ihar Hrachyshka @ 2017-05-17 21:33 UTC (permalink / raw)
To: davem, ja; +Cc: Ihar Hrachyshka, netdev
In-Reply-To: <cover.1495056542.git.ihrachys@redhat.com>
The code is quite involving already to earn a separate function for
itself. If anything, it helps arp_process readability.
Signed-off-by: Ihar Hrachyshka <ihrachys@redhat.com>
---
net/ipv4/arp.c | 35 +++++++++++++++++++++++------------
1 file changed, 23 insertions(+), 12 deletions(-)
diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index d54345a..3f06201 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -641,6 +641,27 @@ void arp_xmit(struct sk_buff *skb)
}
EXPORT_SYMBOL(arp_xmit);
+static bool arp_is_garp(struct net_device *dev, int addr_type,
+ __be16 ar_op,
+ __be32 sip, __be32 tip,
+ unsigned char *sha, unsigned char *tha)
+{
+ bool is_garp = tip == sip && addr_type == RTN_UNICAST;
+
+ /* Unsolicited ARP _replies_ also require target hwaddr to be
+ * the same as source.
+ */
+ if (is_garp && ar_op == htons(ARPOP_REPLY))
+ is_garp =
+ /* IPv4 over IEEE 1394 doesn't provide target
+ * hardware address field in its ARP payload.
+ */
+ tha &&
+ !memcmp(tha, sha, dev->addr_len);
+
+ return is_garp;
+}
+
/*
* Process an arp request.
*/
@@ -844,18 +865,8 @@ static int arp_process(struct net *net, struct sock *sk, struct sk_buff *skb)
It is possible, that this option should be enabled for some
devices (strip is candidate)
*/
- is_garp = tip == sip && addr_type == RTN_UNICAST;
-
- /* Unsolicited ARP _replies_ also require target hwaddr to be
- * the same as source.
- */
- if (is_garp && arp->ar_op == htons(ARPOP_REPLY))
- is_garp =
- /* IPv4 over IEEE 1394 doesn't provide target
- * hardware address field in its ARP payload.
- */
- tha &&
- !memcmp(tha, sha, dev->addr_len);
+ is_garp = arp_is_garp(dev, addr_type, arp->ar_op,
+ sip, tip, sha, tha);
if (!n &&
((arp->ar_op == htons(ARPOP_REPLY) &&
--
2.9.3
^ permalink raw reply related
* [PATCH 2/2] arp: always override existing neigh entries with gratuitous ARP
From: Ihar Hrachyshka @ 2017-05-17 21:33 UTC (permalink / raw)
To: davem, ja; +Cc: Ihar Hrachyshka, netdev
In-Reply-To: <cover.1495056542.git.ihrachys@redhat.com>
Currently, when arp_accept is 1, we always override existing neigh
entries with incoming gratuitous ARP replies. Otherwise, we override
them only if new replies satisfy _locktime_ conditional (packets arrive
not earlier than _locktime_ seconds since the last update to the neigh
entry).
The idea behind locktime is to pick the very first (=> close) reply
received in a unicast burst when ARP proxies are used. This helps to
avoid ARP thrashing where Linux would switch back and forth from one
proxy to another.
This logic has nothing to do with gratuitous ARP replies that are
generally not aligned in time when multiple IP address carriers send
them into network.
This patch enforces overriding of existing neigh entries by all incoming
gratuitous ARP packets, irrespective of their time of arrival. This will
make the kernel honour all incoming gratuitous ARP packets.
Signed-off-by: Ihar Hrachyshka <ihrachys@redhat.com>
---
net/ipv4/arp.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index 3f06201..97ea9d8 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -858,16 +858,16 @@ static int arp_process(struct net *net, struct sock *sk, struct sk_buff *skb)
n = __neigh_lookup(&arp_tbl, &sip, dev, 0);
- if (IN_DEV_ARP_ACCEPT(in_dev)) {
- unsigned int addr_type = inet_addr_type_dev_table(net, dev, sip);
-
- /* Unsolicited ARP is not accepted by default.
- It is possible, that this option should be enabled for some
- devices (strip is candidate)
- */
+ if (n || IN_DEV_ARP_ACCEPT(in_dev)) {
+ addr_type = inet_addr_type_dev_table(net, dev, sip);
is_garp = arp_is_garp(dev, addr_type, arp->ar_op,
sip, tip, sha, tha);
+ }
+ if (IN_DEV_ARP_ACCEPT(in_dev)) {
+ /* It is possible, that this option should be enabled for some
+ * devices (strip is candidate)
+ */
if (!n &&
((arp->ar_op == htons(ARPOP_REPLY) &&
addr_type == RTN_UNICAST) || is_garp))
--
2.9.3
^ permalink raw reply related
* [net-realtek-btcoexist] question about identical code for different branches
From: Gustavo A. R. Silva @ 2017-05-17 21:52 UTC (permalink / raw)
To: Larry Finger, Chaoming Li, Kalle Valo
Cc: linux-wireless, netdev, linux-kernel
Hello everybody,
While looking into Coverity ID 1362263 I ran into the following piece
of code at
drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c:1000:
1000void exhalbtc_set_ant_num(struct rtl_priv *rtlpriv, u8 type, u8 ant_num)
1001{
1002 if (BT_COEX_ANT_TYPE_PG == type) {
1003 gl_bt_coexist.board_info.pg_ant_num = ant_num;
1004 gl_bt_coexist.board_info.btdm_ant_num = ant_num;
1005 /* The antenna position:
1006 * Main (default) or Aux for pgAntNum=2 && btdmAntNum =1.
1007 * The antenna position should be determined by
1008 * auto-detect mechanism.
1009 * The following is assumed to main,
1010 * and those must be modified
1011 * if y auto-detect mechanism is ready
1012 */
1013 if ((gl_bt_coexist.board_info.pg_ant_num == 2) &&
1014 (gl_bt_coexist.board_info.btdm_ant_num == 1))
1015 gl_bt_coexist.board_info.btdm_ant_pos =
1016
BTC_ANTENNA_AT_MAIN_PORT;
1017 else
1018 gl_bt_coexist.board_info.btdm_ant_pos =
1019
BTC_ANTENNA_AT_MAIN_PORT;
1020 } else if (BT_COEX_ANT_TYPE_ANTDIV == type) {
1021 gl_bt_coexist.board_info.btdm_ant_num = ant_num;
1022 gl_bt_coexist.board_info.btdm_ant_pos =
1023
BTC_ANTENNA_AT_MAIN_PORT;
1024 } else if (type == BT_COEX_ANT_TYPE_DETECTED) {
1025 gl_bt_coexist.board_info.btdm_ant_num = ant_num;
1026 if (rtlpriv->cfg->mod_params->ant_sel == 1)
1027 gl_bt_coexist.board_info.btdm_ant_pos =
1028 BTC_ANTENNA_AT_AUX_PORT;
1029 else
1030 gl_bt_coexist.board_info.btdm_ant_pos =
1031 BTC_ANTENNA_AT_MAIN_PORT;
1032 }
1033}
The issue is that lines of code 1015-1016 and 1018-1019 are identical
for different branches.
My question here is if one of those assignments should be modified, or
the entire _if_ statement replaced and the function refactored?
I'd really appreciate any comment on this.
Thank you!
--
Gustavo A. R. Silva
^ permalink raw reply
* [PATCH] net1080: Mark nc_dump_ttl() as __maybe_unused
From: Matthias Kaehlcke @ 2017-05-17 22:17 UTC (permalink / raw)
To: linux-usb-u79uwXL29TY76Z2rM5mHXA
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Matthias Kaehlcke
The function is not used, but it looks useful for debugging. Adding the
attribute fixes the following clang warning:
drivers/net/usb/net1080.c:271:20: error: unused function
'nc_dump_ttl' [-Werror,-Wunused-function]
Signed-off-by: Matthias Kaehlcke <mka-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
drivers/net/usb/net1080.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/usb/net1080.c b/drivers/net/usb/net1080.c
index 4cbdb1307f3e..7ade2119f462 100644
--- a/drivers/net/usb/net1080.c
+++ b/drivers/net/usb/net1080.c
@@ -268,7 +268,7 @@ static inline void nc_dump_status(struct usbnet *dev, u16 status)
#define TTL_OTHER(ttl) (0x00ff & (ttl >> 8))
#define MK_TTL(this,other) ((u16)(((other)<<8)|(0x00ff&(this))))
-static inline void nc_dump_ttl(struct usbnet *dev, u16 ttl)
+static inline void __maybe_unused nc_dump_ttl(struct usbnet *dev, u16 ttl)
{
netif_dbg(dev, link, dev->net, "net1080 %s-%s ttl 0x%x this = %d, other = %d\n",
dev->udev->bus->bus_name, dev->udev->devpath,
--
2.13.0.303.g4ebf302169-goog
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH net v2] selftests/bpf: fix broken build due to types.h
From: Yonghong Song @ 2017-05-17 22:18 UTC (permalink / raw)
To: ast, daniel, davem, netdev; +Cc: kernel-team
Commit 0a5539f66133 ("bpf: Provide a linux/types.h override
for bpf selftests.") caused a build failure for tools/testing/selftest/bpf
because of some missing types:
$ make -C tools/testing/selftests/bpf/
...
In file included from /home/yhs/work/net-next/tools/testing/selftests/bpf/test_pkt_access.c:8:
../../../include/uapi/linux/bpf.h:170:3: error: unknown type name '__aligned_u64'
__aligned_u64 key;
...
/usr/include/linux/swab.h:160:8: error: unknown type name '__always_inline'
static __always_inline __u16 __swab16p(const __u16 *p)
...
The type __aligned_u64 is defined in linux:include/uapi/linux/types.h.
The fix is to copy missing type definition into
tools/testing/selftests/bpf/include/uapi/linux/types.h.
Adding additional include "string.h" resolves __always_inline issue.
Fixes: 0a5539f66133 ("bpf: Provide a linux/types.h override for bpf selftests.")
Signed-off-by: Yonghong Song <yhs@fb.com>
---
tools/testing/selftests/bpf/include/uapi/linux/types.h | 16 ++++++++++++++++
tools/testing/selftests/bpf/test_pkt_access.c | 1 +
2 files changed, 17 insertions(+)
diff --git a/tools/testing/selftests/bpf/include/uapi/linux/types.h b/tools/testing/selftests/bpf/include/uapi/linux/types.h
index fbd16a7..5184184 100644
--- a/tools/testing/selftests/bpf/include/uapi/linux/types.h
+++ b/tools/testing/selftests/bpf/include/uapi/linux/types.h
@@ -3,4 +3,20 @@
#include <asm-generic/int-ll64.h>
+/* copied from linux:include/uapi/linux/types.h */
+#define __bitwise
+typedef __u16 __bitwise __le16;
+typedef __u16 __bitwise __be16;
+typedef __u32 __bitwise __le32;
+typedef __u32 __bitwise __be32;
+typedef __u64 __bitwise __le64;
+typedef __u64 __bitwise __be64;
+
+typedef __u16 __bitwise __sum16;
+typedef __u32 __bitwise __wsum;
+
+#define __aligned_u64 __u64 __attribute__((aligned(8)))
+#define __aligned_be64 __be64 __attribute__((aligned(8)))
+#define __aligned_le64 __le64 __attribute__((aligned(8)))
+
#endif /* _UAPI_LINUX_TYPES_H */
diff --git a/tools/testing/selftests/bpf/test_pkt_access.c b/tools/testing/selftests/bpf/test_pkt_access.c
index 39387bb..6e11ba1 100644
--- a/tools/testing/selftests/bpf/test_pkt_access.c
+++ b/tools/testing/selftests/bpf/test_pkt_access.c
@@ -5,6 +5,7 @@
* License as published by the Free Software Foundation.
*/
#include <stddef.h>
+#include <string.h>
#include <linux/bpf.h>
#include <linux/if_ether.h>
#include <linux/if_packet.h>
--
2.9.3
^ permalink raw reply related
* [PATCH] r8152: Mark usb_ocp_read() as __maybe_unused
From: Matthias Kaehlcke @ 2017-05-17 22:20 UTC (permalink / raw)
To: David S . Miller, hayeswang
Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Matthias Kaehlcke
The function is not used, but is probably kept around for debugging and
symmetry with usb_ocp_write(). Adding the attribute fixes the following
clang warning:
drivers/net/usb/r8152.c:825:5: error: unused function 'usb_ocp_read'
[-Werror,-Wunused-function]
Signed-off-by: Matthias Kaehlcke <mka-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
drivers/net/usb/r8152.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index ddc62cb69be8..12776230ab96 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -840,7 +840,7 @@ int pla_ocp_write(struct r8152 *tp, u16 index, u16 byteen, u16 size, void *data)
return generic_ocp_write(tp, index, byteen, size, data, MCU_TYPE_PLA);
}
-static inline
+static inline __maybe_unused
int usb_ocp_read(struct r8152 *tp, u16 index, u16 size, void *data)
{
return generic_ocp_read(tp, index, size, data, MCU_TYPE_USB);
--
2.13.0.303.g4ebf302169-goog
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [Patch RFC net-next] net: usb: r8152: Fix rx_bytes/tx_bytes to include FCS
From: Florian Fainelli @ 2017-05-17 22:28 UTC (permalink / raw)
To: Andrew Lunn, David Miller; +Cc: netdev, hayeswang, Mario Limonciello
In-Reply-To: <1495052615-14360-1-git-send-email-andrew@lunn.ch>
On 05/17/2017 01:23 PM, Andrew Lunn wrote:
> The statistics counters rx_bytes and tx_bytes don't include the
> Ethernet Frame Check Sequence. The hardware appears to calculate it on
> send, so it is not part of the URB passed to the device, and on
> receive it was not being copied into the skb.
>
> Fix the rx_bytes/tx_bytes counters, and copy the FCS into the skb so
> tools like wireshark can see it.
>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
> RFC for the following points:
>
> I run automatic tests on Ethernet switches supported by DSA. I have a
> test host with many Ethernet interfaces which i connect to switch
> ports. Some are quad Ethernet PCIe cards, using the Intel e1000e
> driver. Some are USB-Ethernet dongles using the asix driver. And i
> have some USB-Ethernet dongles using the r8152. The e1000e, asix and
> DSA itself all give consistent rx_bytes and tx_bytes statistics. The
> r8152 consistently returns counters which are 4 bytes per frame lower,
> which results in some test failures.
>
> Is it defined somewhere what should be included in rx_bytes/tx_bytes?
> Should the FCS be included?
>
> This is considered a user API change? The meaning of the counters are
> going to be slightly different after this patch. I could be breaking
> somebody else's tests :-(
I am afraid that we won't be able to enforce a consistent behavior,
because the HW itself is not consistent, both on the NIC and on the
switch side.
For instance, whether your switch's MIB counters do include FCS, Marvell
tags, or not is going to be highly dependent on how the HW is designed,
whether the MIB counter logic is before, or after in the packet ingress
path.
Some NICs also allow passing the CRC frame up the stack (e.g:
NETIF_F_RXFCS), in which case, we should probably report the FCS as part
of skb->len, because that's what the stack is going to be given.
So I am afraid that as far as your tests are going to work, you will
need to have some logic that knows what kind of switch driver/HW is
used, whether to expect FCS/tag lengths reported, just like you will
have to have the same thing on the initiator of the tests....
> ---
> drivers/net/usb/r8152.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index ddc62cb69be8..4081a2cd8b1b 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
> @@ -1228,7 +1228,7 @@ static void write_bulk_callback(struct urb *urb)
> stats->tx_errors += agg->skb_num;
> } else {
> stats->tx_packets += agg->skb_num;
> - stats->tx_bytes += agg->skb_len;
> + stats->tx_bytes += agg->skb_len + CRC_SIZE;
> }
>
> spin_lock(&tp->tx_lock);
> @@ -1826,7 +1826,6 @@ static int rx_bottom(struct r8152 *tp, int budget)
> if (urb->actual_length < len_used)
> break;
>
> - pkt_len -= CRC_SIZE;
> rx_data += sizeof(struct rx_desc);
>
> skb = napi_alloc_skb(napi, pkt_len);
> @@ -1850,7 +1849,7 @@ static int rx_bottom(struct r8152 *tp, int budget)
> }
>
> find_next_rx:
> - rx_data = rx_agg_align(rx_data + pkt_len + CRC_SIZE);
> + rx_data = rx_agg_align(rx_data + pkt_len);
> rx_desc = (struct rx_desc *)rx_data;
> len_used = (int)(rx_data - (u8 *)agg->head);
> len_used += sizeof(struct rx_desc);
> --
> 2.11.0
> ---
> drivers/net/usb/r8152.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index ddc62cb69be8..4081a2cd8b1b 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
> @@ -1228,7 +1228,7 @@ static void write_bulk_callback(struct urb *urb)
> stats->tx_errors += agg->skb_num;
> } else {
> stats->tx_packets += agg->skb_num;
> - stats->tx_bytes += agg->skb_len;
> + stats->tx_bytes += agg->skb_len + CRC_SIZE;
> }
>
> spin_lock(&tp->tx_lock);
> @@ -1826,7 +1826,6 @@ static int rx_bottom(struct r8152 *tp, int budget)
> if (urb->actual_length < len_used)
> break;
>
> - pkt_len -= CRC_SIZE;
> rx_data += sizeof(struct rx_desc);
>
> skb = napi_alloc_skb(napi, pkt_len);
> @@ -1850,7 +1849,7 @@ static int rx_bottom(struct r8152 *tp, int budget)
> }
>
> find_next_rx:
> - rx_data = rx_agg_align(rx_data + pkt_len + CRC_SIZE);
> + rx_data = rx_agg_align(rx_data + pkt_len);
> rx_desc = (struct rx_desc *)rx_data;
> len_used = (int)(rx_data - (u8 *)agg->head);
> len_used += sizeof(struct rx_desc);
>
--
Florian
^ permalink raw reply
* RE: [PATCH] net/smc: mark as BROKEN due to remote memory exposure
From: Parav Pandit @ 2017-05-17 22:37 UTC (permalink / raw)
To: Doug Ledford, David Miller
Cc: Bart.VanAssche@sandisk.com, torvalds@linux-foundation.org,
hch@lst.de, netdev@vger.kernel.org, linux-rdma@vger.kernel.org,
stable@vger.kernel.org, ubraun@linux.vnet.ibm.com
In-Reply-To: <1495053467.2240.29.camel@redhat.com>
Hi Doug,
> -----Original Message-----
> From: linux-rdma-owner@vger.kernel.org [mailto:linux-rdma-
> owner@vger.kernel.org] On Behalf Of Doug Ledford
> Sent: Wednesday, May 17, 2017 3:38 PM
> To: David Miller <davem@davemloft.net>
> Cc: Bart.VanAssche@sandisk.com; torvalds@linux-foundation.org;
> hch@lst.de; netdev@vger.kernel.org; linux-rdma@vger.kernel.org;
> stable@vger.kernel.org; ubraun@linux.vnet.ibm.com
> Subject: Re: [PATCH] net/smc: mark as BROKEN due to remote memory
> exposure
>
> On Tue, 2017-05-16 at 15:28 -0400, Doug Ledford wrote:
> > I hadn't realized EXPERIMENTAL was gone. Which is too bad, because
> > that's entirely appropriate in this case, and would have had the
> > desired side effect of keeping it out of any non-cutting edge distros
> > and warning people of possible API changes. With EXPERIMENTAL gone,
> > the closest thing we have is drivers/staging, since that tends to
> > imply some of the same consequences. I know you think BROKEN is
> > overly harsh, but I'm not sure we should just do nothing. How about
> > we take a few days to let some of the RDMA people closely review the
> > 143 page
> > (egads!) rfc (http://www.rfc-editor.org/info/rfc7609) to see if we
> > think it can be fixed to use multiple link layers with the existing
> > API in place or if it will require something other than AF_SMC. If we
> > need to break API, then I think we should either fix it ASAP and send
> > that fix to the 4.11 stable series (which probably violates the
> > normative stable patch size/scope) or if the fix will take longer than
> > this kernel cycle, then move it to staging both here and in 4.11
> > stable, and fix it there and then move it back. Something like that
> > would prevent the kind of API flappage we ought not do....
>
> So, I've skimmed the entire RFC, focusing on things were I needed to.
> Here's my take on it:
>
> It would have been better with AF_INET/AF_INET6 and an option to enable
> SMC than AF_SMC. The first implementation simply assumes AF_INET in
> the presence of AF_SMC. When IPv6 support is added, some sort of
> guessing logic will have to be put in place to try and determine if an
> AF_SMC address is actually AF_INET or AF_INET6 since we won't have a
> guaranteed way of telling. Apps can use struct sockaddr_storage as their
> normal element to stick the address into, and could rely on the kernel to
> interpret it properly based on the AF_INET/AF_INET6 differentiation, and
> this breaks that. The RFC gives *some* thought to adding IPv6 in the
> future, but not a lot. It may be that the answer is that in the future, IPv6
> support is enabled by making the IPv6 API be
> AF_INET6 + setsockopt(SMC) or the equivalent. If that's the case, then I
> would suggest making the later API specifically call out AF_INET +
> setsockopt(SMC) be identical to AF_SMC.
>
What are the shortcomings in my proposal [1] which I am reiterating below.
Bart also suggested to define new stream protocol for SMC similar to SCTP.
(a) address family should be either AF_INET or AF_INET6
(b) socket() API to introduce new protocol as PROTO_SMC for in socket() system call.
With this there is no additional setsockop() needed.
With this - user space applications, do getaddrinfo() with hint as
hints.ai_family = AF_INET;
hints.ai_socktype = SOCK_STREAM;
getaddrinfo() returns back both the protocols TCP and SMC.
Famous database application such as Redis client iterates over all entries of getaddrinfo() and establishes connection to servers.
There are few advantages of this interface.
1. No change in any makefile of applications needed - unless user wants to specify explicitly that it wants to force SMC protocol.
2. No need to do LD_LIBRARY_PRELOAD, (which won't work anyway because bind() connect() of SMC checks for AF_INET).
3. No major changes to glibc to process AF_SMC differently
glibc references IPPROTO_TCP at 22 places. (compare to AF_INET at 140+ places).
A lot simpler test matrix for glibc for new protocol
5. No need to recompile applications, as long as getaddrinfo returns all streaming protocols (TCP, SMC)
6. for applications to make use of setsockopt() it needs another knob and hint from other places, which can be avoided because SMC TCP option negotiates with remote end
And representing new protocol as new protocol for a given address family appears correct, compare to setting socket options.
Tools like CRIU or similar tool might find a race conditions - when it queries socket option, SMC was not set, but later on SMC was set, and does incorrect handling.
Setting socket() with SMC protocol makes it easier to understand in this area as well.
I have additional proposal for link groups, resource creation area. I will take that up after this discussion.
[1] https://patchwork.kernel.org/patch/9719375/
> The protocol included a version header in the negotation messages.
> This is good as it allows us to almost immediately start work on version 2
> that fixes the shortcomings of version 1 while maintaining back
> compatibility.
>
> After reading the RFC, I can see why they only support one link layer:
> RoCE. The issue here is that they punted on the hard issue of doing any sort
> of work to determine if security restrictions on the TCP connection should
> also be applied to the RDMA connection. The RFC basically says "the RoCE
> link needs to have the same physical restrictions (vlan) as the TCP/IP link so
> that the security limitations are the same" because they don't do anything
> to check them essentially.
> For v2 of the protocol, and for different link layer support, this is no longer
> sufficient, so there will be significant work to determine the security context
> of specific TCP connections and then make sure that they meet the security
> context of the RDMA links allowed.
>
> Additionally, the same is likely to be true in terms of SELinux options. The
> addition of the IB/OPA link layers will throw a bit of a monkey wrench in
> things because the SELinux control over those links is slightly different than
> a normal TCP/IP SELinux control. For instance, you might create rules about
> processes and ports to make sure that the httpd daemon can only access
> specific ports on TCP/IP, but on IB/OPA you would need to create process
> and P_Key rules as IB/OPA don't have the same port level semantics, and
> it's the P_Key on communications that is enforced network wide, including
> in the switches, so controlling what P_Key a process can send/receive on is
> your best way to limit what a process can do. Translation from one to the
> other might be rather difficult to do in any sort of automated fashion.
>
> There might have to be some additional work to get this to properly
> account items for the RDMA control group elements that were recently
> taken into the kernel.
>
> Finally, the RDMA subsystem is in the process of switching to structured
> option processing similar to how netlink does it. For version 2 of this
> protocol, since it will be interacting with the RDMA core in many ways, will
> be simpler if it switches the on-wire negotiation packets to follow the same
> methods as that will allow reuse of code that it will have to have for
> properly dealing with the RDMA subsystem in the future.
>
> So, I'm fine with it being left as is since you accepted the patch that makes
> note of the memory registration insecurity in the Kconfig text.
> The fact that this is a versioned protocol means that we can fix the things
> we see as being wrong without having to have it all right from the very
> start, it can be done incrementally.
>
> --
> Doug Ledford <dledford@redhat.com>
> GPG KeyID: B826A3330E572FDD
>
> Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" 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
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