* [PATCH] datapath: Fix an error handling path in 'ovs_nla_init_match_and_action()'
From: Christophe JAILLET @ 2017-09-11 19:20 UTC (permalink / raw)
To: pshelar, davem, xiangxia.m.yue
Cc: netdev, dev, linux-kernel, kernel-janitors, Christophe JAILLET
All other error handling paths in this function go through the 'error'
label. This one should do the same.
Fixes: 9cc9a5cb176c ("datapath: Avoid using stack larger than 1024.")
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
I think that the comment above the function could be improved. It looks
like the commit log which has introduced this function.
I'm also not sure that commit 9cc9a5cb176c is of any help. It is
supposed to remove a warning, and I guess it does. But 'ovs_nla_init_match_and_action()'
is called unconditionnaly from 'ovs_flow_cmd_set()'. So even if the stack
used by each function is reduced, the overall stack should be the same, if
not larger.
So this commit sounds like adding a bug where the code was fine and states
to fix an issue but, at the best, only hides it.
Instead of fixing the code with the proposed patch, reverting the initial
commit could also be considered.
---
net/openvswitch/datapath.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index 76cf273a56c7..c3aec6227c91 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -1112,7 +1112,8 @@ static int ovs_nla_init_match_and_action(struct net *net,
if (!a[OVS_FLOW_ATTR_KEY]) {
OVS_NLERR(log,
"Flow key attribute not present in set flow.");
- return -EINVAL;
+ error = -EINVAL;
+ goto error;
}
*acts = get_flow_actions(net, a[OVS_FLOW_ATTR_ACTIONS], key,
--
2.11.0
^ permalink raw reply related
* Re: [PATCH v5 10/10] net: stmmac: dwmac-sun8i: Handle integrated/external MDIOs
From: Corentin Labbe @ 2017-09-11 19:08 UTC (permalink / raw)
To: Andrew Lunn
Cc: robh+dt, mark.rutland, maxime.ripard, wens, linux,
catalin.marinas, will.deacon, peppe.cavallaro, alexandre.torgue,
f.fainelli, netdev, devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <20170911161124.GD27599@lunn.ch>
On Mon, Sep 11, 2017 at 06:11:24PM +0200, Andrew Lunn wrote:
> On Fri, Sep 08, 2017 at 04:28:25PM +0200, Corentin Labbe wrote:
> > On Fri, Sep 08, 2017 at 04:17:36PM +0200, Andrew Lunn wrote:
> > > > > Do you know why the reset times out/fails?
> > > > >
> > > >
> > > > Because there are nothing connected to it.
> > >
> > > That should not be an issue. A read should just return 0xffff. And it
> > > should return 0xffff fast. The timing of the MDIO protocol is fixed. A
> > > read or a write takes a fixed number of cycles, independent of if
> > > there is a device there or not. The bus data line has a pullup, so if
> > > you try to access a missing device, you automatically read 0xffff.
> > >
> >
> > Perhaps, but the reality is that with nothing connected to it, the reset of the MAC timeout.
> > Certainly, the MAC does not support finding no PHY.
>
> Are you sure this is not because of the clock and reset?
>
> + #address-cells = <1>;
> + #size-cells = <0>;
> + int_mii_phy: ethernet-phy@1 {
> + compatible = "ethernet-phy-ieee802.3-c22";
> + reg = <1>;
> + clocks = <&ccu CLK_BUS_EPHY>;
> + resets = <&ccu RST_BUS_EPHY>;
>
> The way you describe it here, the clock and reset are for the PHY. But
> maybe it is actually for the bus? I can understand a bus timing out if
> it has no clock, or it is held in reset. Try enabling the clock and
> reset when the internal bus is selected, not when the PHY on the bus
> is selected.
>
Even with CLK_BUS_EPHY/RST_BUS_EPHY enabled, the MAC reset timeout.
So no the CLK/RST are really for the PHY.
Regards
PS: patch and result with "integrated CLK/RST always on"
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -659,7 +659,7 @@ static int mdio_mux_syscon_switch_fn(int current_child, int desired_child,
struct sunxi_priv_data *gmac = priv->plat->bsp_priv;
u32 reg, val;
int ret = 0;
- bool need_reset = false;
+ bool need_reset = true;
if (current_child ^ desired_child) {
regmap_read(gmac->regmap, SYSCON_EMAC_REG, ®);
@@ -824,7 +824,7 @@ static int sun8i_dwmac_power_internal_phy(struct stmmac_priv *priv)
int ret;
if (!gmac->use_internal_phy)
- return 0;
+ dev_info(priv->device, "IPHY BYPASS\n");
ret = clk_prepare_enable(gmac->ephy_clk);
if (ret) {
[ 18.057162] dwmac-sun8i 1c30000.ethernet: Will use external PHY
[ 18.183789] dwmac-sun8i 1c30000.ethernet: IPHY BYPASS
[ 18.184136] dwmac-sun8i 1c30000.ethernet: Chain mode enabled
[ 18.184158] dwmac-sun8i 1c30000.ethernet: No HW DMA feature register supported
[ 18.184175] dwmac-sun8i 1c30000.ethernet: Normal descriptors
[ 18.184192] dwmac-sun8i 1c30000.ethernet: RX Checksum Offload Engine supported
[ 18.184214] dwmac-sun8i 1c30000.ethernet: COE Type 2
[ 18.184231] dwmac-sun8i 1c30000.ethernet: TX Checksum insertion supported
[ 18.185491] libphy: stmmac: probed
[ 18.188481] libphy: mdio_mux: probed
[ 18.188831] dwmac-sun8i 1c30000.ethernet: Switch mux to internal PHY
[ 18.288981] dwmac-sun8i 1c30000.ethernet: EMAC reset timeout
[ 18.289559] libphy: mdio_mux: probed
[ 18.289629] dwmac-sun8i 1c30000.ethernet: Switch mux to external PHY
[ 20.578316] EXT4-fs (mmcblk0p1): re-mounted. Opts: (null)
[ 31.240650] RTL8211E Gigabit Ethernet 0.1:00: attached PHY driver [RTL8211E Gigabit Ethernet] (mii_bus:phy_addr=0.1:00, irq=POLL)
^ permalink raw reply
* [PATCH v2 net 2/3] lan78xx: Allow EEPROM write for less than MAX_EEPROM_SIZE
From: Nisar.Sayed @ 2017-09-11 18:52 UTC (permalink / raw)
To: davem; +Cc: UNGLinuxDriver, netdev
From: Nisar Sayed <Nisar.Sayed@microchip.com>
Allow EEPROM write for less than MAX_EEPROM_SIZE
Fixes: 55d7de9de6c3 ("Microchip's LAN7800 family USB 2/3 to 10/100/1000 Ethernet device driver")
Signed-off-by: Nisar Sayed <Nisar.Sayed@microchip.com>
---
drivers/net/usb/lan78xx.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index baf91c7..02d64f75 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -1290,8 +1290,8 @@ static int lan78xx_ethtool_set_eeprom(struct net_device *netdev,
/* Allow entire eeprom update only */
if ((ee->magic == LAN78XX_EEPROM_MAGIC) &&
- (ee->offset == 0) &&
- (ee->len == 512) &&
+ (ee->offset >= 0 && ee->offset < MAX_EEPROM_SIZE) &&
+ (ee->len > 0 && (ee->offset + ee->len) <= MAX_EEPROM_SIZE) &&
(data[0] == EEPROM_INDICATOR))
ret = lan78xx_write_raw_eeprom(dev, ee->offset, ee->len, data);
else if ((ee->magic == LAN78XX_OTP_MAGIC) &&
--
1.9.1
^ permalink raw reply related
* [PATCH v2 net 3/3] lan78xx: Use default value loaded from EEPROM/OTP when resetting
From: Nisar.Sayed @ 2017-09-11 18:52 UTC (permalink / raw)
To: davem; +Cc: UNGLinuxDriver, netdev
From: Nisar Sayed <Nisar.Sayed@microchip.com>
Use default value loaded from EEPROM/OTP when resetting
The LAN78xx allows platform configurations to be loaded from EEPROM/OTP.
Ex: When external phy is connected, the MAC can be configured to
have speed, duplex, polarity configurations loadded from the EEPROM/OTP.
Fixes: 55d7de9de6c3 ("Microchip's LAN7800 family USB 2/3 to 10/100/1000 Ethernet device driver")
Signed-off-by: Nisar Sayed <Nisar.Sayed@microchip.com>
---
drivers/net/usb/lan78xx.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index 02d64f75..ffe302a 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -2448,7 +2448,6 @@ static int lan78xx_reset(struct lan78xx_net *dev)
/* LAN7801 only has RGMII mode */
if (dev->chipid == ID_REV_CHIP_ID_7801_)
buf &= ~MAC_CR_GMII_EN_;
- buf |= MAC_CR_AUTO_DUPLEX_ | MAC_CR_AUTO_SPEED_;
ret = lan78xx_write_reg(dev, MAC_CR, buf);
ret = lan78xx_read_reg(dev, MAC_TX, &buf);
--
1.9.1
^ permalink raw reply related
* [PATCH v2 net 1/3] lan78xx: Fix for eeprom read/write when device auto suspend
From: Nisar.Sayed @ 2017-09-11 18:51 UTC (permalink / raw)
To: davem; +Cc: UNGLinuxDriver, netdev
From: Nisar Sayed <Nisar.Sayed@microchip.com>
Fix for eeprom read/write when device auto suspend
Fixes: 55d7de9de6c3 ("Microchip's LAN7800 family USB 2/3 to 10/100/1000 Ethernet device driver")
Signed-off-by: Nisar Sayed <Nisar.Sayed@microchip.com>
---
drivers/net/usb/lan78xx.c | 22 ++++++++++++++++++----
1 file changed, 18 insertions(+), 4 deletions(-)
diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index b99a7fb..baf91c7 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -1265,30 +1265,44 @@ static int lan78xx_ethtool_get_eeprom(struct net_device *netdev,
struct ethtool_eeprom *ee, u8 *data)
{
struct lan78xx_net *dev = netdev_priv(netdev);
+ int ret = -EINVAL;
+
+ if (usb_autopm_get_interface(dev->intf) < 0)
+ return ret;
ee->magic = LAN78XX_EEPROM_MAGIC;
- return lan78xx_read_raw_eeprom(dev, ee->offset, ee->len, data);
+ ret = lan78xx_read_raw_eeprom(dev, ee->offset, ee->len, data);
+
+ usb_autopm_put_interface(dev->intf);
+
+ return ret;
}
static int lan78xx_ethtool_set_eeprom(struct net_device *netdev,
struct ethtool_eeprom *ee, u8 *data)
{
struct lan78xx_net *dev = netdev_priv(netdev);
+ int ret = -EINVAL;
+
+ if (usb_autopm_get_interface(dev->intf) < 0)
+ return ret;
/* Allow entire eeprom update only */
if ((ee->magic == LAN78XX_EEPROM_MAGIC) &&
(ee->offset == 0) &&
(ee->len == 512) &&
(data[0] == EEPROM_INDICATOR))
- return lan78xx_write_raw_eeprom(dev, ee->offset, ee->len, data);
+ ret = lan78xx_write_raw_eeprom(dev, ee->offset, ee->len, data);
else if ((ee->magic == LAN78XX_OTP_MAGIC) &&
(ee->offset == 0) &&
(ee->len == 512) &&
(data[0] == OTP_INDICATOR_1))
- return lan78xx_write_raw_otp(dev, ee->offset, ee->len, data);
+ ret = lan78xx_write_raw_otp(dev, ee->offset, ee->len, data);
- return -EINVAL;
+ usb_autopm_put_interface(dev->intf);
+
+ return ret;
}
static void lan78xx_get_strings(struct net_device *netdev, u32 stringset,
--
1.9.1
^ permalink raw reply related
* [PATCH v2 net 0/3] lan78xx: Fixes to lan78xx driver
From: Nisar.Sayed @ 2017-09-11 18:51 UTC (permalink / raw)
To: davem; +Cc: UNGLinuxDriver, netdev
From: Nisar Sayed <Nisar.Sayed@microchip.com>
This series of patches are for lan78xx driver.
These patches fixes potential issues associated with lan78xx driver
v2
- Added patch version information
- Added fixes tag
- Updated patch description
- Updated changes as per comments
v1
- Spitted patches as per comments
- Dropped "fixed_phy device support" and "Fix for system suspend" changes
Nisar Sayed (3):
Fix for eeprom read/write when device auto suspend
Allow EEPROM write for less than MAX_EEPROM_SIZE
Use default value loaded from EEPROM/OTP when resetting
drivers/net/usb/lan78xx.c | 27 ++++++++++++++++++++-------
1 file changed, 20 insertions(+), 7 deletions(-)
--
1.9.1
^ permalink raw reply
* Re: [PATCH] net/sched: fix pointer check in gen_handle
From: Cong Wang @ 2017-09-11 18:05 UTC (permalink / raw)
To: Josh Hunt
Cc: David Miller, Jamal Hadi Salim, Jiri Pirko,
Linux Kernel Network Developers
In-Reply-To: <1505072930-13049-1-git-send-email-johunt@akamai.com>
On Sun, Sep 10, 2017 at 12:48 PM, Josh Hunt <johunt@akamai.com> wrote:
> Fixes sparse warning about pointer in gen_handle:
> net/sched/cls_rsvp.h:392:40: warning: Using plain integer as NULL pointer
>
> Fixes: 8113c095672f6 ("net_sched: use void pointer for filter handle")
> Signed-off-by: Josh Hunt <johunt@akamai.com>
Acked-by: Cong Wang <xiyou.wangcong@gmail.com>
^ permalink raw reply
* Re: [RFC PATCH] net: Introduce a socket option to enable picking tx queue based on rx queue.
From: Alexander Duyck @ 2017-09-11 17:48 UTC (permalink / raw)
To: Samudrala, Sridhar; +Cc: Duyck, Alexander H, Netdev
In-Reply-To: <91a48bc5-57d9-db09-78c0-98a49b414a28@intel.com>
On Mon, Sep 11, 2017 at 9:48 AM, Samudrala, Sridhar
<sridhar.samudrala@intel.com> wrote:
>
>
> On 9/9/2017 6:28 PM, Alexander Duyck wrote:
>>
>> On Thu, Aug 31, 2017 at 4:27 PM, Sridhar Samudrala
>> <sridhar.samudrala@intel.com> wrote:
>>>
>>> This patch introduces a new socket option SO_SYMMETRIC_QUEUES that can be
>>> used
>>> to enable symmetric tx and rx queues on a socket.
>>>
>>> This option is specifically useful for epoll based multi threaded
>>> workloads
>>> where each thread handles packets received on a single RX queue . In this
>>> model,
>>> we have noticed that it helps to send the packets on the same TX queue
>>> corresponding to the queue-pair associated with the RX queue specifically
>>> when
>>> busy poll is enabled with epoll().
>>>
>>> Two new fields are added to struct sock_common to cache the last rx
>>> ifindex and
>>> the rx queue in the receive path of an SKB. __netdev_pick_tx() returns
>>> the cached
>>> rx queue when this option is enabled and the TX is happening on the same
>>> device.
>>>
>>> Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
>>> ---
>>> include/net/request_sock.h | 1 +
>>> include/net/sock.h | 17 +++++++++++++++++
>>> include/uapi/asm-generic/socket.h | 2 ++
>>> net/core/dev.c | 8 +++++++-
>>> net/core/sock.c | 10 ++++++++++
>>> net/ipv4/tcp_input.c | 1 +
>>> net/ipv4/tcp_ipv4.c | 1 +
>>> net/ipv4/tcp_minisocks.c | 1 +
>>> 8 files changed, 40 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/include/net/request_sock.h b/include/net/request_sock.h
>>> index 23e2205..c3bc12e 100644
>>> --- a/include/net/request_sock.h
>>> +++ b/include/net/request_sock.h
>>> @@ -100,6 +100,7 @@ static inline struct sock *req_to_sk(struct
>>> request_sock *req)
>>> req_to_sk(req)->sk_prot = sk_listener->sk_prot;
>>> sk_node_init(&req_to_sk(req)->sk_node);
>>> sk_tx_queue_clear(req_to_sk(req));
>>> + req_to_sk(req)->sk_symmetric_queues =
>>> sk_listener->sk_symmetric_queues;
>>> req->saved_syn = NULL;
>>> refcount_set(&req->rsk_refcnt, 0);
>>>
>>> diff --git a/include/net/sock.h b/include/net/sock.h
>>> index 03a3625..3421809 100644
>>> --- a/include/net/sock.h
>>> +++ b/include/net/sock.h
>>> @@ -138,11 +138,14 @@ void SOCK_DEBUG(const struct sock *sk, const char
>>> *msg, ...)
>>> * @skc_node: main hash linkage for various protocol lookup tables
>>> * @skc_nulls_node: main hash linkage for TCP/UDP/UDP-Lite protocol
>>> * @skc_tx_queue_mapping: tx queue number for this connection
>>> + * @skc_rx_queue_mapping: rx queue number for this connection
>>> + * @skc_rx_ifindex: rx ifindex for this connection
>>> * @skc_flags: place holder for sk_flags
>>> * %SO_LINGER (l_onoff), %SO_BROADCAST, %SO_KEEPALIVE,
>>> * %SO_OOBINLINE settings, %SO_TIMESTAMPING settings
>>> * @skc_incoming_cpu: record/match cpu processing incoming packets
>>> * @skc_refcnt: reference count
>>> + * @skc_symmetric_queues: symmetric tx/rx queues
>>> *
>>> * This is the minimal network layer representation of sockets, the
>>> header
>>> * for struct sock and struct inet_timewait_sock.
>>> @@ -177,6 +180,7 @@ struct sock_common {
>>> unsigned char skc_reuseport:1;
>>> unsigned char skc_ipv6only:1;
>>> unsigned char skc_net_refcnt:1;
>>> + unsigned char skc_symmetric_queues:1;
>>> int skc_bound_dev_if;
>>> union {
>>> struct hlist_node skc_bind_node;
>>> @@ -214,6 +218,8 @@ struct sock_common {
>>> struct hlist_nulls_node skc_nulls_node;
>>> };
>>> int skc_tx_queue_mapping;
>>> + int skc_rx_queue_mapping;
>>> + int skc_rx_ifindex;
>>> union {
>>> int skc_incoming_cpu;
>>> u32 skc_rcv_wnd;
>>> @@ -324,6 +330,8 @@ struct sock {
>>> #define sk_nulls_node __sk_common.skc_nulls_node
>>> #define sk_refcnt __sk_common.skc_refcnt
>>> #define sk_tx_queue_mapping __sk_common.skc_tx_queue_mapping
>>> +#define sk_rx_queue_mapping __sk_common.skc_rx_queue_mapping
>>> +#define sk_rx_ifindex __sk_common.skc_rx_ifindex
>>>
>>> #define sk_dontcopy_begin __sk_common.skc_dontcopy_begin
>>> #define sk_dontcopy_end __sk_common.skc_dontcopy_end
>>> @@ -340,6 +348,7 @@ struct sock {
>>> #define sk_reuseport __sk_common.skc_reuseport
>>> #define sk_ipv6only __sk_common.skc_ipv6only
>>> #define sk_net_refcnt __sk_common.skc_net_refcnt
>>> +#define sk_symmetric_queues __sk_common.skc_symmetric_queues
>>> #define sk_bound_dev_if __sk_common.skc_bound_dev_if
>>> #define sk_bind_node __sk_common.skc_bind_node
>>> #define sk_prot __sk_common.skc_prot
>>> @@ -1676,6 +1685,14 @@ static inline int sk_tx_queue_get(const struct
>>> sock *sk)
>>> return sk ? sk->sk_tx_queue_mapping : -1;
>>> }
>>>
>>> +static inline void sk_mark_rx_queue(struct sock *sk, struct sk_buff
>>> *skb)
>>> +{
>>> + if (sk->sk_symmetric_queues) {
>>> + sk->sk_rx_ifindex = skb->skb_iif;
>>> + sk->sk_rx_queue_mapping = skb_get_rx_queue(skb);
>>> + }
>>> +}
>>> +
>>> static inline void sk_set_socket(struct sock *sk, struct socket *sock)
>>> {
>>> sk_tx_queue_clear(sk);
>>> diff --git a/include/uapi/asm-generic/socket.h
>>> b/include/uapi/asm-generic/socket.h
>>> index e47c9e4..f6b416e 100644
>>> --- a/include/uapi/asm-generic/socket.h
>>> +++ b/include/uapi/asm-generic/socket.h
>>> @@ -106,4 +106,6 @@
>>>
>>> #define SO_ZEROCOPY 60
>>>
>>> +#define SO_SYMMETRIC_QUEUES 61
>>> +
>>> #endif /* __ASM_GENERIC_SOCKET_H */
>>> diff --git a/net/core/dev.c b/net/core/dev.c
>>> index 270b547..d96cda8 100644
>>> --- a/net/core/dev.c
>>> +++ b/net/core/dev.c
>>> @@ -3322,7 +3322,13 @@ static u16 __netdev_pick_tx(struct net_device
>>> *dev, struct sk_buff *skb)
>>>
>>> if (queue_index < 0 || skb->ooo_okay ||
>>> queue_index >= dev->real_num_tx_queues) {
>>> - int new_index = get_xps_queue(dev, skb);
>>> + int new_index = -1;
>>> +
>>> + if (sk && sk->sk_symmetric_queues && dev->ifindex ==
>>> sk->sk_rx_ifindex)
>>> + new_index = sk->sk_rx_queue_mapping;
>>> +
>>> + if (new_index < 0 || new_index >=
>>> dev->real_num_tx_queues)
>>> + new_index = get_xps_queue(dev, skb);
>>>
>>> if (new_index < 0)
>>> new_index = skb_tx_hash(dev, skb);
>>
>> So one thing I am not sure about is if we should be overriding XPS. It
>> might make sense to instead place this after XPS so that if the root
>> user configures it then it applies, otherwise if the socket is
>> requesting symmetric queues you could fall back to that, and then
>> finally just use hashing as the final solution for distributing the
>> workload.
>
> Isn't XPS on by default and all the devices that support XPS setup XPS maps
> as part of
> the initialization?
> Are you suggesting that the root user needs to disable XPS on the specific
> queues before an
> application can use this option to enable symmetric queues?
>
XPS and this symmetric queue logic won't really play well together
since they are both attempting to do the same thing but from different
ends. Some sort of priority needs to be defined, and I would place the
request of the kernel/root user above the request of a socket and/or
application.
I guess it comes down to if we let the Tx CPU pick the queue or Rx
queue, and I would argue with an XPS configuration in place the kernel
is requesting that the Tx CPU sets the Tx queue, not the Rx/Tx hash or
Rx queue. This is similar to how we handle this for routing today as
XPS will also override the Rx to Tx mapping used there.
>>
>> That way if somebody decides to reserve queues for some sort of
>> specific traffic like AF_PACKET then they can configure the Tx via
>> XPS, configure the Rx via RSS redirection table reprogramming, and
>> then setup a filters on the hardware to direct the traffic they want
>> to the queues that are running AF_PACKET.
>>
>>> diff --git a/net/core/sock.c b/net/core/sock.c
>>> index 9b7b6bb..3876cce 100644
>>> --- a/net/core/sock.c
>>> +++ b/net/core/sock.c
>>> @@ -1059,6 +1059,10 @@ int sock_setsockopt(struct socket *sock, int
>>> level, int optname,
>>> sock_valbool_flag(sk, SOCK_ZEROCOPY, valbool);
>>> break;
>>>
>>> + case SO_SYMMETRIC_QUEUES:
>>> + sk->sk_symmetric_queues = valbool;
>>> + break;
>>> +
>>> default:
>>> ret = -ENOPROTOOPT;
>>> break;
>>> @@ -1391,6 +1395,10 @@ int sock_getsockopt(struct socket *sock, int
>>> level, int optname,
>>> v.val = sock_flag(sk, SOCK_ZEROCOPY);
>>> break;
>>>
>>> + case SO_SYMMETRIC_QUEUES:
>>> + v.val = sk->sk_symmetric_queues;
>>> + break;
>>> +
>>> default:
>>> /* We implement the SO_SNDLOWAT etc to not be settable
>>> * (1003.1g 7).
>>> @@ -2738,6 +2746,8 @@ void sock_init_data(struct socket *sock, struct
>>> sock *sk)
>>> sk->sk_max_pacing_rate = ~0U;
>>> sk->sk_pacing_rate = ~0U;
>>> sk->sk_incoming_cpu = -1;
>>> + sk->sk_rx_ifindex = -1;
>>> + sk->sk_rx_queue_mapping = -1;
>>> /*
>>> * Before updating sk_refcnt, we must commit prior changes to
>>> memory
>>> * (Documentation/RCU/rculist_nulls.txt for details)
>>> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
>>> index c5d7656..12381e0 100644
>>> --- a/net/ipv4/tcp_input.c
>>> +++ b/net/ipv4/tcp_input.c
>>> @@ -6356,6 +6356,7 @@ int tcp_conn_request(struct request_sock_ops
>>> *rsk_ops,
>>> tcp_rsk(req)->snt_isn = isn;
>>> tcp_rsk(req)->txhash = net_tx_rndhash();
>>> tcp_openreq_init_rwin(req, sk, dst);
>>> + sk_mark_rx_queue(req_to_sk(req), skb);
>>> if (!want_cookie) {
>>> tcp_reqsk_record_syn(sk, req, skb);
>>> fastopen_sk = tcp_try_fastopen(sk, skb, req, &foc);
>>> diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
>>> index a63486a..82f9af4 100644
>>> --- a/net/ipv4/tcp_ipv4.c
>>> +++ b/net/ipv4/tcp_ipv4.c
>>> @@ -1450,6 +1450,7 @@ int tcp_v4_do_rcv(struct sock *sk, struct sk_buff
>>> *skb)
>>>
>>> sock_rps_save_rxhash(sk, skb);
>>> sk_mark_napi_id(sk, skb);
>>> + sk_mark_rx_queue(sk, skb);
>>> if (dst) {
>>> if (inet_sk(sk)->rx_dst_ifindex != skb->skb_iif
>>> ||
>>> !dst->ops->check(dst, 0)) {
>>> diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
>>> index 188a6f3..2b5efd5 100644
>>> --- a/net/ipv4/tcp_minisocks.c
>>> +++ b/net/ipv4/tcp_minisocks.c
>>> @@ -809,6 +809,7 @@ int tcp_child_process(struct sock *parent, struct
>>> sock *child,
>>>
>>> /* record NAPI ID of child */
>>> sk_mark_napi_id(child, skb);
>>> + sk_mark_rx_queue(child, skb);
>>>
>>> tcp_segs_in(tcp_sk(child), skb);
>>> if (!sock_owned_by_user(child)) {
>>> --
>>> 1.8.3.1
>>>
>
^ permalink raw reply
* [PATCH v2 net] smsc95xx: Configure pause time to 0xffff when tx flow control enabled
From: Nisar.Sayed @ 2017-09-11 17:43 UTC (permalink / raw)
To: davem; +Cc: UNGLinuxDriver, netdev, steve.glendinning
From: Nisar Sayed <Nisar.Sayed@microchip.com>
Configure pause time to 0xffff when tx flow control enabled
Set pause time to 0xffff in the pause frame to indicate the
partner to stop sending the packets. When RX buffer frees up,
the device sends pause frame with pause time zero for partner to
resume transmission.
Fixes: 2f7ca802bdae ("Add SMSC LAN9500 USB2.0 10/100 ethernet adapter driver")
Signed-off-by: Nisar Sayed <Nisar.Sayed@microchip.com>
---
v0 -> v1:
* Added patch description in detail.
v1 -> v2:
* Added fixes tag
---
drivers/net/usb/smsc95xx.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 340c134..309b88a 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -526,7 +526,7 @@ static void smsc95xx_set_multicast(struct net_device *netdev)
static int smsc95xx_phy_update_flowcontrol(struct usbnet *dev, u8 duplex,
u16 lcladv, u16 rmtadv)
{
- u32 flow, afc_cfg = 0;
+ u32 flow = 0, afc_cfg;
int ret = smsc95xx_read_reg(dev, AFC_CFG, &afc_cfg);
if (ret < 0)
@@ -537,20 +537,19 @@ static int smsc95xx_phy_update_flowcontrol(struct usbnet *dev, u8 duplex,
if (cap & FLOW_CTRL_RX)
flow = 0xFFFF0002;
- else
- flow = 0;
- if (cap & FLOW_CTRL_TX)
+ if (cap & FLOW_CTRL_TX) {
afc_cfg |= 0xF;
- else
+ flow |= 0xFFFF0000;
+ } else {
afc_cfg &= ~0xF;
+ }
netif_dbg(dev, link, dev->net, "rx pause %s, tx pause %s\n",
cap & FLOW_CTRL_RX ? "enabled" : "disabled",
cap & FLOW_CTRL_TX ? "enabled" : "disabled");
} else {
netif_dbg(dev, link, dev->net, "half duplex\n");
- flow = 0;
afc_cfg |= 0xF;
}
--
1.9.1
^ permalink raw reply related
* Re: [patch net] mlxsw: spectrum: Fix EEPROM access in case of SFP/SFP+
From: David Miller @ 2017-09-11 17:41 UTC (permalink / raw)
To: jiri; +Cc: netdev, arkadis, idosch, flokli, mlxsw
In-Reply-To: <20170911074226.2020-1-jiri@resnulli.us>
From: Jiri Pirko <jiri@resnulli.us>
Date: Mon, 11 Sep 2017 09:42:26 +0200
> From: Arkadi Sharshevsky <arkadis@mellanox.com>
>
> The current code does not handle correctly the access to the upper page
> in case of SFP/SFP+ EEPROM. In that case the offset should be local
> and the I2C address should be changed.
>
> Fixes: 2ea109039cd3 ("mlxsw: spectrum: Add support for access cable info via ethtool")
> Reported-by: Florian Klink <flokli@flokli.de>
> Signed-off-by: Arkadi Sharshevsky <arkadis@mellanox.com>
> Reviewed-by: Ido Schimmel <idosch@mellanox.com>
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH net] MAINTAINERS: Remove Yuval Mintz from maintainers list
From: David Miller @ 2017-09-11 17:38 UTC (permalink / raw)
To: aelior; +Cc: netdev, everest-linux-l2
In-Reply-To: <20170911140513.24387-1-aelior@cavium.com>
From: <aelior@cavium.com>
Date: Mon, 11 Sep 2017 17:05:13 +0300
> From: Ariel Elior <aelior@cavium.com>
>
> Remove Yuval from maintaining the bnx2x & qed* modules as he is no longer
> working for the company. Thanks Yuval for your huge contributions and
> tireless efforts over the many years and various companies.
>
> Ariel
All patches must have a proper signoff, therefore please resubmit this
with one.
^ permalink raw reply
* RE: [PATCH v1 net] smsc95xx: Configure pause time to 0xffff when tx flow control enabled
From: Nisar.Sayed @ 2017-09-11 17:38 UTC (permalink / raw)
To: andrew; +Cc: davem, UNGLinuxDriver, netdev, steve.glendinning
In-Reply-To: <20170911140220.GC27599@lunn.ch>
> On Mon, Sep 11, 2017 at 12:32:10PM +0000, Nisar.Sayed@microchip.com
> wrote:
> > From: Nisar Sayed <Nisar.Sayed@microchip.com>
> >
> > Configure pause time to 0xffff when tx flow control enabled
> >
> > Set pause time to 0xffff in the pause frame to indicate the partner to
> > stop sending the packets. When RX buffer frees up, the device sends
> > pause frame with pause time zero for partner to resume transmission.
>
> Hi Nisar
>
> Thanks for the updated description. Since you are posting this for net, not
> net-next, could you add a fixes: tag?
>
> Thanks
> Andrew
Thanks Andrew, yes I will add and will submit next revision
- Nisar
^ permalink raw reply
* Re: [PATCH net] netlink: access nlk groups safely in netlink bind and getname
From: Cong Wang @ 2017-09-11 17:35 UTC (permalink / raw)
To: Xin Long; +Cc: network dev, David Miller, Florian Westphal, David Herrmann
In-Reply-To: <CADvbK_dBm1b97eFqGrEp1EZN8Sfozm3PJ1MhqEZK+R3FB47-ZQ@mail.gmail.com>
On Sun, Sep 10, 2017 at 4:45 AM, Xin Long <lucien.xin@gmail.com> wrote:
> On Sat, Sep 9, 2017 at 7:35 AM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>> On Tue, Sep 5, 2017 at 8:53 PM, Xin Long <lucien.xin@gmail.com> wrote:
>>> Now there is no lock protecting nlk ngroups/groups' accessing in
>>> netlink bind and getname. It's safe from nlk groups' setting in
>>> netlink_release, but not from netlink_realloc_groups called by
>>> netlink_setsockopt.
>>>
>>> netlink_lock_table is needed in both netlink bind and getname when
>>> accessing nlk groups.
>>
>> This looks very odd.
>>
>> netlink_lock_table() should be protecting nl_table, why
>> it also protects nlk->groups?? For me it looks like you
>> need lock_sock() instead.
> I believe netlink_lock_table might be only used to protect nl_table
> at the beginning and surely lock_sock is better here. Thanks.
>
> But can you explain why netlink_lock_table() was also used in
> netlink_getsockopt NETLINK_LIST_MEMBERSHIPS ? or it
> was just a mistake ?
No, it is fine but not necessary, because netlink_realloc_groups()
doesn't change nl_table, it only changes nlk->groups. So we
don't have take the global write lock, the lock sock makes more
sense here, same for your bind() and getname() case.
^ permalink raw reply
* Re: [PATCH net] phy: mvebu-cp110: checking for NULL instead of IS_ERR()
From: David Miller @ 2017-09-11 17:29 UTC (permalink / raw)
To: kishon; +Cc: dan.carpenter, antoine.tenart, netdev, kernel-janitors,
linux-kernel
In-Reply-To: <1bc9c815-08ba-1095-eb6d-a753b3d0c755@ti.com>
From: Kishon Vijay Abraham I <kishon@ti.com>
Date: Mon, 11 Sep 2017 12:54:47 +0530
> On Saturday 09 September 2017 09:39 AM, David Miller wrote:
>> Yeah let's sort this out before I apply this fix to my tree.
>
> I'll take this fix in linux-phy tree after -rc1 is tagged if that's okay with you.
No problem, although I don't see why one would delay bug fixes... ever.
^ permalink raw reply
* Re: [patch net] mlxsw: spectrum: Fix EEPROM access in case of SFP/SFP+
From: Jiri Pirko @ 2017-09-11 17:27 UTC (permalink / raw)
To: Andrew Lunn; +Cc: netdev, davem, arkadis, idosch, flokli, mlxsw
In-Reply-To: <20170911135826.GB27599@lunn.ch>
Mon, Sep 11, 2017 at 03:58:26PM CEST, andrew@lunn.ch wrote:
>On Mon, Sep 11, 2017 at 09:42:26AM +0200, Jiri Pirko wrote:
>> From: Arkadi Sharshevsky <arkadis@mellanox.com>
>>
>> The current code does not handle correctly the access to the upper page
>> in case of SFP/SFP+ EEPROM. In that case the offset should be local
>> and the I2C address should be changed.
>
>Shame you cannot/didn't expose the i2c bus as a linux i2c bus. The AT24 code
We cannot.
^ permalink raw reply
* Re: Kernel 4.13.0-rc4-next-20170811 - IP Routing / Forwarding performance vs Core/RSS number / HT on
From: Paweł Staszewski @ 2017-09-11 16:57 UTC (permalink / raw)
To: Eric Dumazet
Cc: Paolo Abeni, Jesper Dangaard Brouer,
Linux Kernel Network Developers, Alexander Duyck
In-Reply-To: <709ff9f4-39f9-b5e5-206d-b16edd8c7320@itcare.pl>
Tested with connectx-5
Without patch
10Mpps - > 16 cores used
PerfTop: 66258 irqs/sec kernel:99.3% exact: 0.0% [4000Hz
cycles], (all, 32 CPUs)
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
10.12% [kernel] [k] do_raw_spin_lock
6.31% [kernel] [k] fib_table_lookup
6.12% [kernel] [k] mlx5e_handle_rx_cqe_mpwrq
4.90% [kernel] [k] rt_cache_valid
3.99% [kernel] [k] mlx5e_xmit
3.03% [kernel] [k] ip_rcv
2.68% [kernel] [k] __netif_receive_skb_core
2.54% [kernel] [k] skb_dst_force
2.41% [kernel] [k] ip_finish_output2
2.21% [kernel] [k] __build_skb
2.03% [kernel] [k] __dev_queue_xmit
1.96% [kernel] [k] mlx5e_txwqe_complete
1.79% [kernel] [k] ipt_do_table
1.78% [kernel] [k] inet_gro_receive
1.69% [kernel] [k] ip_forward
1.66% [kernel] [k] udp_v4_early_demux
1.65% [kernel] [k] dst_release
1.56% [kernel] [k] ip_rcv_finish
1.45% [kernel] [k] dev_gro_receive
1.45% [kernel] [k] netif_skb_features
1.39% [kernel] [k] mlx5e_poll_tx_cq
1.35% [kernel] [k] mlx5e_txwqe_build_dsegs
1.35% [kernel] [k] ip_route_input_rcu
1.15% [kernel] [k] dev_hard_start_xmit
1.12% [kernel] [k] napi_gro_receive
1.07% [kernel] [k] netif_receive_skb_internal
0.98% [kernel] [k] sch_direct_xmit
0.95% [kernel] [k] kmem_cache_alloc
0.89% [kernel] [k] read_tsc
0.88% [kernel] [k] mlx5e_build_rx_skb
0.86% [kernel] [k] mlx5_cqwq_get_cqe
0.82% [kernel] [k] page_frag_free
0.78% [kernel] [k] __local_bh_enable_ip
0.69% [kernel] [k] skb_network_protocol
0.68% [kernel] [k] __netif_receive_skb
0.67% [kernel] [k] vlan_dev_hard_start_xmit
0.65% [kernel] [k] mlx5e_poll_rx_cq
0.65% [kernel] [k] validate_xmit_skb
0.60% [kernel] [k] eth_type_trans
0.60% [kernel] [k] deliver_ptype_list_skb
0.60% [kernel] [k] fib_validate_source
0.55% [kernel] [k] eth_header
0.53% [kernel] [k] netdev_pick_tx
0.53% [kernel] [k] __napi_alloc_skb
0.51% [kernel] [k] __udp4_lib_lookup
0.50% [kernel] [k] eth_type_vlan
0.49% [kernel] [k] ip_output
0.49% [kernel] [k] page_frag_alloc
0.49% [kernel] [k] ip_finish_output
0.48% [kernel] [k] neigh_connected_output
0.45% [kernel] [k] nf_hook_slow
0.44% [kernel] [k] udp4_gro_receive
0.39% [kernel] [k] mlx5e_features_check
0.39% [kernel] [k] mlx5e_napi_poll
0.37% [kernel] [k] __jhash_nwords
0.37% [kernel] [k] udp_gro_receive
0.36% [kernel] [k] swiotlb_map_page
0.33% [kernel] [k] mlx5_cqwq_get_wqe
0.33% [kernel] [k] __netdev_pick_tx
0.29% [kernel] [k] ktime_get_with_offset
0.29% [kernel] [k] get_dma_ops
0.29% [kernel] [k] validate_xmit_skb_list
0.26% [kernel] [k] vlan_passthru_hard_header
0.26% [kernel] [k] __udp4_lib_lookup_skb
0.24% [kernel] [k] get_dma_ops
0.24% [kernel] [k] skb_release_data
0.23% [kernel] [k] ip_forward_finish
0.23% [kernel] [k] kmem_cache_free_bulk
0.23% [kernel] [k] timekeeping_get_ns
0.22% [kernel] [k] ip_skb_dst_mtu
0.21% [kernel] [k] compound_head
0.20% [kernel] [k] skb_gro_reset_offset
0.20% [kernel] [k] is_swiotlb_buffer
0.19% [kernel] [k] __net_timestamp.isra.90
0.19% [kernel] [k] dst_metric.constprop.61
0.18% [kernel] [k] skb_orphan_frags.constprop.126
0.18% [kernel] [k] _kfree_skb_defer
0.18% [kernel] [k] irq_entries_start
0.17% [kernel] [k] dev_hard_header.constprop.54
0.17% [kernel] [k] dma_mapping_error
0.17% [kernel] [k] neigh_resolve_output
With patch
12Mpps -> 16 cores
PerfTop: 66209 irqs/sec kernel:99.3% exact: 0.0% [4000Hz
cycles], (all, 32 CPUs)
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
10.67% [kernel] [k] do_raw_spin_lock
6.96% [kernel] [k] fib_table_lookup
6.53% [kernel] [k] mlx5e_handle_rx_cqe_mpwrq
4.17% [kernel] [k] mlx5e_xmit
3.22% [kernel] [k] ip_rcv
3.07% [kernel] [k] __netif_receive_skb_core
2.86% [kernel] [k] __dev_queue_xmit
2.36% [kernel] [k] __build_skb
2.33% [kernel] [k] ip_forward
2.05% [kernel] [k] mlx5e_txwqe_complete
2.02% [kernel] [k] ip_finish_output2
2.00% [kernel] [k] ipt_do_table
1.84% [kernel] [k] ip_rcv_finish
1.83% [kernel] [k] inet_gro_receive
1.80% [kernel] [k] udp_v4_early_demux
1.61% [kernel] [k] dev_gro_receive
1.55% [kernel] [k] netif_skb_features
1.52% [kernel] [k] mlx5e_txwqe_build_dsegs
1.47% [kernel] [k] mlx5e_poll_tx_cq
1.39% [kernel] [k] ip_route_input_rcu
1.38% [kernel] [k] dev_hard_start_xmit
1.17% [kernel] [k] netif_receive_skb_internal
1.16% [kernel] [k] napi_gro_receive
1.03% [kernel] [k] kmem_cache_alloc
1.02% [kernel] [k] sch_direct_xmit
0.97% [kernel] [k] read_tsc
0.94% [kernel] [k] page_frag_free
0.91% [kernel] [k] mlx5_cqwq_get_cqe
0.90% [kernel] [k] mlx5e_build_rx_skb
0.89% [kernel] [k] skb_network_protocol
0.83% [kernel] [k] __local_bh_enable_ip
0.79% [kernel] [k] validate_xmit_skb
0.77% [kernel] [k] vlan_dev_hard_start_xmit
0.74% [kernel] [k] __netif_receive_skb
0.72% [kernel] [k] mlx5e_poll_rx_cq
0.70% [kernel] [k] netdev_pick_tx
0.69% [kernel] [k] eth_type_vlan
0.68% [kernel] [k] __netdev_pick_tx
0.66% [kernel] [k] nf_hook_slow
0.65% [kernel] [k] deliver_ptype_list_skb
0.62% [kernel] [k] fib_validate_source
0.61% [kernel] [k] eth_header
0.60% [kernel] [k] eth_type_trans
0.59% [kernel] [k] __udp4_lib_lookup
0.58% [kernel] [k] __napi_alloc_skb
0.53% [kernel] [k] ip_finish_output
0.51% [kernel] [k] neigh_connected_output
0.50% [kernel] [k] ip_output
0.50% [kernel] [k] rt_cache_valid
0.44% [kernel] [k] udp4_gro_receive
0.43% [kernel] [k] mlx5e_napi_poll
0.40% [kernel] [k] udp_gro_receive
0.40% [kernel] [k] page_frag_alloc
0.40% [kernel] [k] __jhash_nwords
0.39% [kernel] [k] swiotlb_map_page
0.38% [kernel] [k] mlx5_cqwq_get_wqe
0.36% [kernel] [k] mlx5e_features_check
0.32% [kernel] [k] get_dma_ops
0.31% [kernel] [k] ktime_get_with_offset
0.31% [kernel] [k] validate_xmit_skb_list
0.28% [kernel] [k] vlan_passthru_hard_header
0.28% [kernel] [k] get_dma_ops
0.27% [kernel] [k] __udp4_lib_lookup_skb
0.26% [kernel] [k] skb_gro_reset_offset
0.25% [kernel] [k] skb_release_data
0.25% [kernel] [k] timekeeping_get_ns
0.24% [kernel] [k] kmem_cache_free_bulk
0.24% [kernel] [k] ip_forward_finish
0.23% [kernel] [k] compound_head
0.23% [kernel] [k] ip_skb_dst_mtu
0.22% [kernel] [k] __net_timestamp.isra.90
0.22% [kernel] [k] is_swiotlb_buffer
0.21% [kernel] [k] neigh_resolve_output
0.21% [kernel] [k] dst_metric.constprop.61
0.20% [kernel] [k] skb_orphan_frags.constprop.126
0.20% [kernel] [k] irq_entries_start
0.19% [kernel] [k] mlx5e_calc_min_inline
0.19% [kernel] [k] dev_hard_header.constprop.54
0.19% [kernel] [k] _kfree_skb_defer
0.18% [kernel] [k] _raw_spin_lock
0.18% [kernel] [k] ip_route_input_noref
W dniu 2017-09-09 o 11:03, Paweł Staszewski pisze:
> Hi
>
>
> Are there any plans to have this fix normally in kernel ?
>
> Or it is mostly only hack - not longterm fix and need to be different ?
>
>
> All tests that was done shows that without this patch there is about
> 20-30% network forwarding performance degradation when using vlan
> interfaces
>
>
> Thanks
> Paweł
>
>
>
> W dniu 2017-08-15 o 03:17, Eric Dumazet pisze:
>> On Mon, 2017-08-14 at 18:07 -0700, Eric Dumazet wrote:
>>
>>> Or try to hack the IFF_XMIT_DST_RELEASE flag on the vlan netdev.
>> Something like :
>>
>> diff --git a/net/8021q/vlan_netlink.c b/net/8021q/vlan_netlink.c
>> index
>> 5e831de3103e2f7092c7fa15534def403bc62fb4..9472de846d5c0960996261cb2843032847fa4bf7
>> 100644
>> --- a/net/8021q/vlan_netlink.c
>> +++ b/net/8021q/vlan_netlink.c
>> @@ -143,6 +143,7 @@ static int vlan_newlink(struct net *src_net,
>> struct net_device *dev,
>> vlan->vlan_proto = proto;
>> vlan->vlan_id = nla_get_u16(data[IFLA_VLAN_ID]);
>> vlan->real_dev = real_dev;
>> + dev->priv_flags |= (real_dev->priv_flags & IFF_XMIT_DST_RELEASE);
>> vlan->flags = VLAN_FLAG_REORDER_HDR;
>> err = vlan_check_real_dev(real_dev, vlan->vlan_proto,
>> vlan->vlan_id);
>>
>>
>>
>>
>
>
^ permalink raw reply
* Re: [RFC PATCH] net: Introduce a socket option to enable picking tx queue based on rx queue.
From: Samudrala, Sridhar @ 2017-09-11 16:49 UTC (permalink / raw)
To: Tom Herbert; +Cc: Alexander Duyck, Linux Kernel Network Developers
In-Reply-To: <CALx6S371o0Oz9M7jmQWLuG2n3J0NGYGxZ_+okOkFL3x_+GB+2w@mail.gmail.com>
On 9/10/2017 8:19 AM, Tom Herbert wrote:
> On Thu, Aug 31, 2017 at 4:27 PM, Sridhar Samudrala
> <sridhar.samudrala@intel.com> wrote:
>> This patch introduces a new socket option SO_SYMMETRIC_QUEUES that can be used
>> to enable symmetric tx and rx queues on a socket.
>>
>> This option is specifically useful for epoll based multi threaded workloads
>> where each thread handles packets received on a single RX queue . In this model,
>> we have noticed that it helps to send the packets on the same TX queue
>> corresponding to the queue-pair associated with the RX queue specifically when
>> busy poll is enabled with epoll().
>>
>> Two new fields are added to struct sock_common to cache the last rx ifindex and
>> the rx queue in the receive path of an SKB. __netdev_pick_tx() returns the cached
>> rx queue when this option is enabled and the TX is happening on the same device.
>>
>> Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
>> ---
>> include/net/request_sock.h | 1 +
>> include/net/sock.h | 17 +++++++++++++++++
>> include/uapi/asm-generic/socket.h | 2 ++
>> net/core/dev.c | 8 +++++++-
>> net/core/sock.c | 10 ++++++++++
>> net/ipv4/tcp_input.c | 1 +
>> net/ipv4/tcp_ipv4.c | 1 +
>> net/ipv4/tcp_minisocks.c | 1 +
>> 8 files changed, 40 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/net/request_sock.h b/include/net/request_sock.h
>> index 23e2205..c3bc12e 100644
>> --- a/include/net/request_sock.h
>> +++ b/include/net/request_sock.h
>> @@ -100,6 +100,7 @@ static inline struct sock *req_to_sk(struct request_sock *req)
>> req_to_sk(req)->sk_prot = sk_listener->sk_prot;
>> sk_node_init(&req_to_sk(req)->sk_node);
>> sk_tx_queue_clear(req_to_sk(req));
>> + req_to_sk(req)->sk_symmetric_queues = sk_listener->sk_symmetric_queues;
>> req->saved_syn = NULL;
>> refcount_set(&req->rsk_refcnt, 0);
>>
>> diff --git a/include/net/sock.h b/include/net/sock.h
>> index 03a3625..3421809 100644
>> --- a/include/net/sock.h
>> +++ b/include/net/sock.h
>> @@ -138,11 +138,14 @@ void SOCK_DEBUG(const struct sock *sk, const char *msg, ...)
>> * @skc_node: main hash linkage for various protocol lookup tables
>> * @skc_nulls_node: main hash linkage for TCP/UDP/UDP-Lite protocol
>> * @skc_tx_queue_mapping: tx queue number for this connection
>> + * @skc_rx_queue_mapping: rx queue number for this connection
>> + * @skc_rx_ifindex: rx ifindex for this connection
>> * @skc_flags: place holder for sk_flags
>> * %SO_LINGER (l_onoff), %SO_BROADCAST, %SO_KEEPALIVE,
>> * %SO_OOBINLINE settings, %SO_TIMESTAMPING settings
>> * @skc_incoming_cpu: record/match cpu processing incoming packets
>> * @skc_refcnt: reference count
>> + * @skc_symmetric_queues: symmetric tx/rx queues
>> *
>> * This is the minimal network layer representation of sockets, the header
>> * for struct sock and struct inet_timewait_sock.
>> @@ -177,6 +180,7 @@ struct sock_common {
>> unsigned char skc_reuseport:1;
>> unsigned char skc_ipv6only:1;
>> unsigned char skc_net_refcnt:1;
>> + unsigned char skc_symmetric_queues:1;
>> int skc_bound_dev_if;
>> union {
>> struct hlist_node skc_bind_node;
>> @@ -214,6 +218,8 @@ struct sock_common {
>> struct hlist_nulls_node skc_nulls_node;
>> };
>> int skc_tx_queue_mapping;
>> + int skc_rx_queue_mapping;
>> + int skc_rx_ifindex;
>> union {
>> int skc_incoming_cpu;
>> u32 skc_rcv_wnd;
>> @@ -324,6 +330,8 @@ struct sock {
>> #define sk_nulls_node __sk_common.skc_nulls_node
>> #define sk_refcnt __sk_common.skc_refcnt
>> #define sk_tx_queue_mapping __sk_common.skc_tx_queue_mapping
>> +#define sk_rx_queue_mapping __sk_common.skc_rx_queue_mapping
>> +#define sk_rx_ifindex __sk_common.skc_rx_ifindex
>>
>> #define sk_dontcopy_begin __sk_common.skc_dontcopy_begin
>> #define sk_dontcopy_end __sk_common.skc_dontcopy_end
>> @@ -340,6 +348,7 @@ struct sock {
>> #define sk_reuseport __sk_common.skc_reuseport
>> #define sk_ipv6only __sk_common.skc_ipv6only
>> #define sk_net_refcnt __sk_common.skc_net_refcnt
>> +#define sk_symmetric_queues __sk_common.skc_symmetric_queues
>> #define sk_bound_dev_if __sk_common.skc_bound_dev_if
>> #define sk_bind_node __sk_common.skc_bind_node
>> #define sk_prot __sk_common.skc_prot
>> @@ -1676,6 +1685,14 @@ static inline int sk_tx_queue_get(const struct sock *sk)
>> return sk ? sk->sk_tx_queue_mapping : -1;
>> }
>>
>> +static inline void sk_mark_rx_queue(struct sock *sk, struct sk_buff *skb)
>> +{
>> + if (sk->sk_symmetric_queues) {
>> + sk->sk_rx_ifindex = skb->skb_iif;
>> + sk->sk_rx_queue_mapping = skb_get_rx_queue(skb);
>> + }
>> +}
>> +
>> static inline void sk_set_socket(struct sock *sk, struct socket *sock)
>> {
>> sk_tx_queue_clear(sk);
>> diff --git a/include/uapi/asm-generic/socket.h b/include/uapi/asm-generic/socket.h
>> index e47c9e4..f6b416e 100644
>> --- a/include/uapi/asm-generic/socket.h
>> +++ b/include/uapi/asm-generic/socket.h
>> @@ -106,4 +106,6 @@
>>
>> #define SO_ZEROCOPY 60
>>
>> +#define SO_SYMMETRIC_QUEUES 61
>> +
>> #endif /* __ASM_GENERIC_SOCKET_H */
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index 270b547..d96cda8 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -3322,7 +3322,13 @@ static u16 __netdev_pick_tx(struct net_device *dev, struct sk_buff *skb)
>>
>> if (queue_index < 0 || skb->ooo_okay ||
>> queue_index >= dev->real_num_tx_queues) {
>> - int new_index = get_xps_queue(dev, skb);
>> + int new_index = -1;
>> +
>> + if (sk && sk->sk_symmetric_queues && dev->ifindex == sk->sk_rx_ifindex)
>> + new_index = sk->sk_rx_queue_mapping;
>> +
>> + if (new_index < 0 || new_index >= dev->real_num_tx_queues)
>> + new_index = get_xps_queue(dev, skb);
> This enforces that notion of queue pairs which is not universal
> concept to NICs. There are many devices and instances where we
> purposely avoid having a 1-1 relationship between rx and tx queues.
Yes. This patch assumes that TX and RX queues come in pairs.
> An alternative might be to create a rx queue to tx queue map, add the
> rx queue argument to get_xps_queue, and then that function can
> consider the mapping. The administrator can configure the mapping as
> appropriate and can select which rx queues are subject to the mapping.
This alternative looks much cleaner and doesn't require the apps to
configure the
queues. Do we need to support 1 to many rx to tx queue mappings?
For our symmetric queues usecase, where a single application thread is
associated with
1 queue-pair, 1-1 mapping is sufficient.
Do you see any usecase where it is useful to support 1-many mappings?
I guess i can add a sysfs entry per rx-queue to setup a tx-queue OR
tx-queue-map.
>
>> if (new_index < 0)
>> new_index = skb_tx_hash(dev, skb);
>> diff --git a/net/core/sock.c b/net/core/sock.c
>> index 9b7b6bb..3876cce 100644
>> --- a/net/core/sock.c
>> +++ b/net/core/sock.c
>> @@ -1059,6 +1059,10 @@ int sock_setsockopt(struct socket *sock, int level, int optname,
>> sock_valbool_flag(sk, SOCK_ZEROCOPY, valbool);
>> break;
>>
>> + case SO_SYMMETRIC_QUEUES:
>> + sk->sk_symmetric_queues = valbool;
>> + break;
>> +
> Allowing users control over this seems problematic to me. The intent
> of packet steering is to provide good loading across the whole
> systems, not just for individual applications. Exposing this control
> makes that mission harder.
Sure. If we can do this on a per rxqueue basis that can be configured by
the administrator,
it would be a better option.
>
>> default:
>> ret = -ENOPROTOOPT;
>> break;
>> @@ -1391,6 +1395,10 @@ int sock_getsockopt(struct socket *sock, int level, int optname,
>> v.val = sock_flag(sk, SOCK_ZEROCOPY);
>> break;
>>
>> + case SO_SYMMETRIC_QUEUES:
>> + v.val = sk->sk_symmetric_queues;
>> + break;
>> +
>> default:
>> /* We implement the SO_SNDLOWAT etc to not be settable
>> * (1003.1g 7).
>> @@ -2738,6 +2746,8 @@ void sock_init_data(struct socket *sock, struct sock *sk)
>> sk->sk_max_pacing_rate = ~0U;
>> sk->sk_pacing_rate = ~0U;
>> sk->sk_incoming_cpu = -1;
>> + sk->sk_rx_ifindex = -1;
>> + sk->sk_rx_queue_mapping = -1;
>> /*
>> * Before updating sk_refcnt, we must commit prior changes to memory
>> * (Documentation/RCU/rculist_nulls.txt for details)
>> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
>> index c5d7656..12381e0 100644
>> --- a/net/ipv4/tcp_input.c
>> +++ b/net/ipv4/tcp_input.c
>> @@ -6356,6 +6356,7 @@ int tcp_conn_request(struct request_sock_ops *rsk_ops,
>> tcp_rsk(req)->snt_isn = isn;
>> tcp_rsk(req)->txhash = net_tx_rndhash();
>> tcp_openreq_init_rwin(req, sk, dst);
>> + sk_mark_rx_queue(req_to_sk(req), skb);
>> if (!want_cookie) {
>> tcp_reqsk_record_syn(sk, req, skb);
>> fastopen_sk = tcp_try_fastopen(sk, skb, req, &foc);
>> diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
>> index a63486a..82f9af4 100644
>> --- a/net/ipv4/tcp_ipv4.c
>> +++ b/net/ipv4/tcp_ipv4.c
>> @@ -1450,6 +1450,7 @@ int tcp_v4_do_rcv(struct sock *sk, struct sk_buff *skb)
>>
>> sock_rps_save_rxhash(sk, skb);
>> sk_mark_napi_id(sk, skb);
>> + sk_mark_rx_queue(sk, skb);
> This could be part of sock_rps_save_rxhash instead of new functions in
> core receive path. That could be renamed sock_save_rx_info or
> something like that. UDP support is also lacking with this patch so
> that gets solved by a common function also.
Sure. Will look into re factoring sock_rps_save_rxhash() to save the rx
queue info.
>
>> if (dst) {
>> if (inet_sk(sk)->rx_dst_ifindex != skb->skb_iif ||
>> !dst->ops->check(dst, 0)) {
>> diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
>> index 188a6f3..2b5efd5 100644
>> --- a/net/ipv4/tcp_minisocks.c
>> +++ b/net/ipv4/tcp_minisocks.c
>> @@ -809,6 +809,7 @@ int tcp_child_process(struct sock *parent, struct sock *child,
>>
>> /* record NAPI ID of child */
>> sk_mark_napi_id(child, skb);
>> + sk_mark_rx_queue(child, skb);
>>
>> tcp_segs_in(tcp_sk(child), skb);
>> if (!sock_owned_by_user(child)) {
>> --
>> 1.8.3.1
>>
^ permalink raw reply
* Re: [RFC PATCH] net: Introduce a socket option to enable picking tx queue based on rx queue.
From: Samudrala, Sridhar @ 2017-09-11 16:48 UTC (permalink / raw)
To: Alexander Duyck; +Cc: Duyck, Alexander H, Netdev
In-Reply-To: <CAKgT0UfZed3_3KBmKbMbcmdsA+ctFRUCu8jp_rCnatC9AMv__g@mail.gmail.com>
On 9/9/2017 6:28 PM, Alexander Duyck wrote:
> On Thu, Aug 31, 2017 at 4:27 PM, Sridhar Samudrala
> <sridhar.samudrala@intel.com> wrote:
>> This patch introduces a new socket option SO_SYMMETRIC_QUEUES that can be used
>> to enable symmetric tx and rx queues on a socket.
>>
>> This option is specifically useful for epoll based multi threaded workloads
>> where each thread handles packets received on a single RX queue . In this model,
>> we have noticed that it helps to send the packets on the same TX queue
>> corresponding to the queue-pair associated with the RX queue specifically when
>> busy poll is enabled with epoll().
>>
>> Two new fields are added to struct sock_common to cache the last rx ifindex and
>> the rx queue in the receive path of an SKB. __netdev_pick_tx() returns the cached
>> rx queue when this option is enabled and the TX is happening on the same device.
>>
>> Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
>> ---
>> include/net/request_sock.h | 1 +
>> include/net/sock.h | 17 +++++++++++++++++
>> include/uapi/asm-generic/socket.h | 2 ++
>> net/core/dev.c | 8 +++++++-
>> net/core/sock.c | 10 ++++++++++
>> net/ipv4/tcp_input.c | 1 +
>> net/ipv4/tcp_ipv4.c | 1 +
>> net/ipv4/tcp_minisocks.c | 1 +
>> 8 files changed, 40 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/net/request_sock.h b/include/net/request_sock.h
>> index 23e2205..c3bc12e 100644
>> --- a/include/net/request_sock.h
>> +++ b/include/net/request_sock.h
>> @@ -100,6 +100,7 @@ static inline struct sock *req_to_sk(struct request_sock *req)
>> req_to_sk(req)->sk_prot = sk_listener->sk_prot;
>> sk_node_init(&req_to_sk(req)->sk_node);
>> sk_tx_queue_clear(req_to_sk(req));
>> + req_to_sk(req)->sk_symmetric_queues = sk_listener->sk_symmetric_queues;
>> req->saved_syn = NULL;
>> refcount_set(&req->rsk_refcnt, 0);
>>
>> diff --git a/include/net/sock.h b/include/net/sock.h
>> index 03a3625..3421809 100644
>> --- a/include/net/sock.h
>> +++ b/include/net/sock.h
>> @@ -138,11 +138,14 @@ void SOCK_DEBUG(const struct sock *sk, const char *msg, ...)
>> * @skc_node: main hash linkage for various protocol lookup tables
>> * @skc_nulls_node: main hash linkage for TCP/UDP/UDP-Lite protocol
>> * @skc_tx_queue_mapping: tx queue number for this connection
>> + * @skc_rx_queue_mapping: rx queue number for this connection
>> + * @skc_rx_ifindex: rx ifindex for this connection
>> * @skc_flags: place holder for sk_flags
>> * %SO_LINGER (l_onoff), %SO_BROADCAST, %SO_KEEPALIVE,
>> * %SO_OOBINLINE settings, %SO_TIMESTAMPING settings
>> * @skc_incoming_cpu: record/match cpu processing incoming packets
>> * @skc_refcnt: reference count
>> + * @skc_symmetric_queues: symmetric tx/rx queues
>> *
>> * This is the minimal network layer representation of sockets, the header
>> * for struct sock and struct inet_timewait_sock.
>> @@ -177,6 +180,7 @@ struct sock_common {
>> unsigned char skc_reuseport:1;
>> unsigned char skc_ipv6only:1;
>> unsigned char skc_net_refcnt:1;
>> + unsigned char skc_symmetric_queues:1;
>> int skc_bound_dev_if;
>> union {
>> struct hlist_node skc_bind_node;
>> @@ -214,6 +218,8 @@ struct sock_common {
>> struct hlist_nulls_node skc_nulls_node;
>> };
>> int skc_tx_queue_mapping;
>> + int skc_rx_queue_mapping;
>> + int skc_rx_ifindex;
>> union {
>> int skc_incoming_cpu;
>> u32 skc_rcv_wnd;
>> @@ -324,6 +330,8 @@ struct sock {
>> #define sk_nulls_node __sk_common.skc_nulls_node
>> #define sk_refcnt __sk_common.skc_refcnt
>> #define sk_tx_queue_mapping __sk_common.skc_tx_queue_mapping
>> +#define sk_rx_queue_mapping __sk_common.skc_rx_queue_mapping
>> +#define sk_rx_ifindex __sk_common.skc_rx_ifindex
>>
>> #define sk_dontcopy_begin __sk_common.skc_dontcopy_begin
>> #define sk_dontcopy_end __sk_common.skc_dontcopy_end
>> @@ -340,6 +348,7 @@ struct sock {
>> #define sk_reuseport __sk_common.skc_reuseport
>> #define sk_ipv6only __sk_common.skc_ipv6only
>> #define sk_net_refcnt __sk_common.skc_net_refcnt
>> +#define sk_symmetric_queues __sk_common.skc_symmetric_queues
>> #define sk_bound_dev_if __sk_common.skc_bound_dev_if
>> #define sk_bind_node __sk_common.skc_bind_node
>> #define sk_prot __sk_common.skc_prot
>> @@ -1676,6 +1685,14 @@ static inline int sk_tx_queue_get(const struct sock *sk)
>> return sk ? sk->sk_tx_queue_mapping : -1;
>> }
>>
>> +static inline void sk_mark_rx_queue(struct sock *sk, struct sk_buff *skb)
>> +{
>> + if (sk->sk_symmetric_queues) {
>> + sk->sk_rx_ifindex = skb->skb_iif;
>> + sk->sk_rx_queue_mapping = skb_get_rx_queue(skb);
>> + }
>> +}
>> +
>> static inline void sk_set_socket(struct sock *sk, struct socket *sock)
>> {
>> sk_tx_queue_clear(sk);
>> diff --git a/include/uapi/asm-generic/socket.h b/include/uapi/asm-generic/socket.h
>> index e47c9e4..f6b416e 100644
>> --- a/include/uapi/asm-generic/socket.h
>> +++ b/include/uapi/asm-generic/socket.h
>> @@ -106,4 +106,6 @@
>>
>> #define SO_ZEROCOPY 60
>>
>> +#define SO_SYMMETRIC_QUEUES 61
>> +
>> #endif /* __ASM_GENERIC_SOCKET_H */
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index 270b547..d96cda8 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -3322,7 +3322,13 @@ static u16 __netdev_pick_tx(struct net_device *dev, struct sk_buff *skb)
>>
>> if (queue_index < 0 || skb->ooo_okay ||
>> queue_index >= dev->real_num_tx_queues) {
>> - int new_index = get_xps_queue(dev, skb);
>> + int new_index = -1;
>> +
>> + if (sk && sk->sk_symmetric_queues && dev->ifindex == sk->sk_rx_ifindex)
>> + new_index = sk->sk_rx_queue_mapping;
>> +
>> + if (new_index < 0 || new_index >= dev->real_num_tx_queues)
>> + new_index = get_xps_queue(dev, skb);
>>
>> if (new_index < 0)
>> new_index = skb_tx_hash(dev, skb);
> So one thing I am not sure about is if we should be overriding XPS. It
> might make sense to instead place this after XPS so that if the root
> user configures it then it applies, otherwise if the socket is
> requesting symmetric queues you could fall back to that, and then
> finally just use hashing as the final solution for distributing the
> workload.
Isn't XPS on by default and all the devices that support XPS setup XPS
maps as part of
the initialization?
Are you suggesting that the root user needs to disable XPS on the
specific queues before an
application can use this option to enable symmetric queues?
>
> That way if somebody decides to reserve queues for some sort of
> specific traffic like AF_PACKET then they can configure the Tx via
> XPS, configure the Rx via RSS redirection table reprogramming, and
> then setup a filters on the hardware to direct the traffic they want
> to the queues that are running AF_PACKET.
>
>> diff --git a/net/core/sock.c b/net/core/sock.c
>> index 9b7b6bb..3876cce 100644
>> --- a/net/core/sock.c
>> +++ b/net/core/sock.c
>> @@ -1059,6 +1059,10 @@ int sock_setsockopt(struct socket *sock, int level, int optname,
>> sock_valbool_flag(sk, SOCK_ZEROCOPY, valbool);
>> break;
>>
>> + case SO_SYMMETRIC_QUEUES:
>> + sk->sk_symmetric_queues = valbool;
>> + break;
>> +
>> default:
>> ret = -ENOPROTOOPT;
>> break;
>> @@ -1391,6 +1395,10 @@ int sock_getsockopt(struct socket *sock, int level, int optname,
>> v.val = sock_flag(sk, SOCK_ZEROCOPY);
>> break;
>>
>> + case SO_SYMMETRIC_QUEUES:
>> + v.val = sk->sk_symmetric_queues;
>> + break;
>> +
>> default:
>> /* We implement the SO_SNDLOWAT etc to not be settable
>> * (1003.1g 7).
>> @@ -2738,6 +2746,8 @@ void sock_init_data(struct socket *sock, struct sock *sk)
>> sk->sk_max_pacing_rate = ~0U;
>> sk->sk_pacing_rate = ~0U;
>> sk->sk_incoming_cpu = -1;
>> + sk->sk_rx_ifindex = -1;
>> + sk->sk_rx_queue_mapping = -1;
>> /*
>> * Before updating sk_refcnt, we must commit prior changes to memory
>> * (Documentation/RCU/rculist_nulls.txt for details)
>> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
>> index c5d7656..12381e0 100644
>> --- a/net/ipv4/tcp_input.c
>> +++ b/net/ipv4/tcp_input.c
>> @@ -6356,6 +6356,7 @@ int tcp_conn_request(struct request_sock_ops *rsk_ops,
>> tcp_rsk(req)->snt_isn = isn;
>> tcp_rsk(req)->txhash = net_tx_rndhash();
>> tcp_openreq_init_rwin(req, sk, dst);
>> + sk_mark_rx_queue(req_to_sk(req), skb);
>> if (!want_cookie) {
>> tcp_reqsk_record_syn(sk, req, skb);
>> fastopen_sk = tcp_try_fastopen(sk, skb, req, &foc);
>> diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
>> index a63486a..82f9af4 100644
>> --- a/net/ipv4/tcp_ipv4.c
>> +++ b/net/ipv4/tcp_ipv4.c
>> @@ -1450,6 +1450,7 @@ int tcp_v4_do_rcv(struct sock *sk, struct sk_buff *skb)
>>
>> sock_rps_save_rxhash(sk, skb);
>> sk_mark_napi_id(sk, skb);
>> + sk_mark_rx_queue(sk, skb);
>> if (dst) {
>> if (inet_sk(sk)->rx_dst_ifindex != skb->skb_iif ||
>> !dst->ops->check(dst, 0)) {
>> diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
>> index 188a6f3..2b5efd5 100644
>> --- a/net/ipv4/tcp_minisocks.c
>> +++ b/net/ipv4/tcp_minisocks.c
>> @@ -809,6 +809,7 @@ int tcp_child_process(struct sock *parent, struct sock *child,
>>
>> /* record NAPI ID of child */
>> sk_mark_napi_id(child, skb);
>> + sk_mark_rx_queue(child, skb);
>>
>> tcp_segs_in(tcp_sk(child), skb);
>> if (!sock_owned_by_user(child)) {
>> --
>> 1.8.3.1
>>
^ permalink raw reply
* Re: [PATCH] net: bonding: Fix transmit load balancing in balance-alb mode if specified by sysfs
From: Nikolay Aleksandrov @ 2017-09-11 16:30 UTC (permalink / raw)
To: Mahesh Bandewar (महेश बंडेवार)
Cc: Kosuke Tatsukawa, Jay Vosburgh, Veaceslav Falico, Andy Gospodarek,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
Reinis Rozitis
In-Reply-To: <CAF2d9jhEQCGuP3jX=cca-1VSmXbZtfCBN7xHnBfiw+_Ch=ihzw@mail.gmail.com>
On 11 September 2017 19:07:33 EEST, "Mahesh Bandewar (महेश बंडेवार)" <maheshb@google.com> wrote:
>On Sat, Sep 9, 2017 at 4:28 AM, Nikolay Aleksandrov
><nikolay@cumulusnetworks.com> wrote:
>> On 07/09/17 01:47, Kosuke Tatsukawa wrote:
>>> Commit cbf5ecb30560 ("net: bonding: Fix transmit load balancing in
>>> balance-alb mode") tried to fix transmit dynamic load balancing in
>>> balance-alb mode, which wasn't working after commit 8b426dc54cf4
>>> ("bonding: remove hardcoded value").
>>>
>>> It turned out that my previous patch only fixed the case when
>>> balance-alb was specified as bonding module parameter, and not when
>>> balance-alb mode was set using /sys/class/net/*/bonding/mode (the
>most
>>> common usage). In the latter case, tlb_dynamic_lb was set up
>according
>>> to the default mode of the bonding interface, which happens to be
>>> balance-rr.
>>>
>>> This additional patch addresses this issue by setting up
>tlb_dynamic_lb
>>> to 1 if "mode" is set to balance-alb through the sysfs interface.
>>>
>>> I didn't add code to change tlb_balance_lb back to the default value
>for
>>> other modes, because "mode" is usually set up only once during
>>> initialization, and it's not worthwhile to change the static
>variable
>>> bonding_defaults in bond_main.c to a global variable just for this
>>> purpose.
>>>
>>> Commit 8b426dc54cf4 also changes the value of tlb_dynamic_lb for
>>> balance-tlb mode if it is set up using the sysfs interface. I
>didn't
>>> change that behavior, because the value of tlb_balance_lb can be
>changed
>>> using the sysfs interface for balance-tlb, and I didn't like
>changing
>>> the default value back and forth for balance-tlb.
>>>
>>> As for balance-alb, /sys/class/net/*/bonding/tlb_balance_lb cannot
>be
>>> written to. However, I think balance-alb with tlb_dynamic_lb set to
>0
>>> is not an intended usage, so there is little use making it writable
>at
>>> this moment.
>>>
>>> Fixes: 8b426dc54cf4 ("bonding: remove hardcoded value")
>>> Reported-by: Reinis Rozitis <r@roze.lv>
>>> Signed-off-by: Kosuke Tatsukawa <tatsu@ab.jp.nec.com>
>>> Cc: stable@vger.kernel.org # v4.12+
>>> ---
>>> drivers/net/bonding/bond_options.c | 3 +++
>>> 1 files changed, 3 insertions(+), 0 deletions(-)
>>>
>>
>> This fix is simpler and more suitable for -net, it fixes the case
>where
>> we switch to ALB mode with tlb_dynamic_lb = 0. After it is in I'll
>fix the
>> default tlb_dynamic_lb issue and restore the original behaviour.
>>
>Changing tlb_dyanamic_lb to initialize always is also safe for -net
>and can go in before or after this change (no dependency on this
>change as such)
I never said it was unsafe or dependent, it is simply my preference to wait. :-)
If you need it sooner feel free to post it.
>
>> Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
>Acked-by: Mahesh Bandewar <maheshb@google.com>
>>
>>
^ permalink raw reply
* Re: [PATCH v5 10/10] net: stmmac: dwmac-sun8i: Handle integrated/external MDIOs
From: Andrew Lunn @ 2017-09-11 16:11 UTC (permalink / raw)
To: Corentin Labbe
Cc: robh+dt, mark.rutland, maxime.ripard, wens, linux,
catalin.marinas, will.deacon, peppe.cavallaro, alexandre.torgue,
f.fainelli, netdev, devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <20170908142825.GC3037@Red>
On Fri, Sep 08, 2017 at 04:28:25PM +0200, Corentin Labbe wrote:
> On Fri, Sep 08, 2017 at 04:17:36PM +0200, Andrew Lunn wrote:
> > > > Do you know why the reset times out/fails?
> > > >
> > >
> > > Because there are nothing connected to it.
> >
> > That should not be an issue. A read should just return 0xffff. And it
> > should return 0xffff fast. The timing of the MDIO protocol is fixed. A
> > read or a write takes a fixed number of cycles, independent of if
> > there is a device there or not. The bus data line has a pullup, so if
> > you try to access a missing device, you automatically read 0xffff.
> >
>
> Perhaps, but the reality is that with nothing connected to it, the reset of the MAC timeout.
> Certainly, the MAC does not support finding no PHY.
Are you sure this is not because of the clock and reset?
+ #address-cells = <1>;
+ #size-cells = <0>;
+ int_mii_phy: ethernet-phy@1 {
+ compatible = "ethernet-phy-ieee802.3-c22";
+ reg = <1>;
+ clocks = <&ccu CLK_BUS_EPHY>;
+ resets = <&ccu RST_BUS_EPHY>;
The way you describe it here, the clock and reset are for the PHY. But
maybe it is actually for the bus? I can understand a bus timing out if
it has no clock, or it is held in reset. Try enabling the clock and
reset when the internal bus is selected, not when the PHY on the bus
is selected.
Andrew
^ permalink raw reply
* Re: [PATCH] net: bonding: Fix transmit load balancing in balance-alb mode if specified by sysfs
From: Mahesh Bandewar (महेश बंडेवार) @ 2017-09-11 16:07 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: Kosuke Tatsukawa, Jay Vosburgh, Veaceslav Falico, Andy Gospodarek,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
Reinis Rozitis
In-Reply-To: <adab6b74-574b-74cb-09ea-3b5090113ca6@cumulusnetworks.com>
On Sat, Sep 9, 2017 at 4:28 AM, Nikolay Aleksandrov
<nikolay@cumulusnetworks.com> wrote:
> On 07/09/17 01:47, Kosuke Tatsukawa wrote:
>> Commit cbf5ecb30560 ("net: bonding: Fix transmit load balancing in
>> balance-alb mode") tried to fix transmit dynamic load balancing in
>> balance-alb mode, which wasn't working after commit 8b426dc54cf4
>> ("bonding: remove hardcoded value").
>>
>> It turned out that my previous patch only fixed the case when
>> balance-alb was specified as bonding module parameter, and not when
>> balance-alb mode was set using /sys/class/net/*/bonding/mode (the most
>> common usage). In the latter case, tlb_dynamic_lb was set up according
>> to the default mode of the bonding interface, which happens to be
>> balance-rr.
>>
>> This additional patch addresses this issue by setting up tlb_dynamic_lb
>> to 1 if "mode" is set to balance-alb through the sysfs interface.
>>
>> I didn't add code to change tlb_balance_lb back to the default value for
>> other modes, because "mode" is usually set up only once during
>> initialization, and it's not worthwhile to change the static variable
>> bonding_defaults in bond_main.c to a global variable just for this
>> purpose.
>>
>> Commit 8b426dc54cf4 also changes the value of tlb_dynamic_lb for
>> balance-tlb mode if it is set up using the sysfs interface. I didn't
>> change that behavior, because the value of tlb_balance_lb can be changed
>> using the sysfs interface for balance-tlb, and I didn't like changing
>> the default value back and forth for balance-tlb.
>>
>> As for balance-alb, /sys/class/net/*/bonding/tlb_balance_lb cannot be
>> written to. However, I think balance-alb with tlb_dynamic_lb set to 0
>> is not an intended usage, so there is little use making it writable at
>> this moment.
>>
>> Fixes: 8b426dc54cf4 ("bonding: remove hardcoded value")
>> Reported-by: Reinis Rozitis <r@roze.lv>
>> Signed-off-by: Kosuke Tatsukawa <tatsu@ab.jp.nec.com>
>> Cc: stable@vger.kernel.org # v4.12+
>> ---
>> drivers/net/bonding/bond_options.c | 3 +++
>> 1 files changed, 3 insertions(+), 0 deletions(-)
>>
>
> This fix is simpler and more suitable for -net, it fixes the case where
> we switch to ALB mode with tlb_dynamic_lb = 0. After it is in I'll fix the
> default tlb_dynamic_lb issue and restore the original behaviour.
>
Changing tlb_dyanamic_lb to initialize always is also safe for -net
and can go in before or after this change (no dependency on this
change as such)
> Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Acked-by: Mahesh Bandewar <maheshb@google.com>
>
>
^ permalink raw reply
* Re: [PATCH] tcp: TCP_USER_TIMEOUT can not work in tcp_probe_timer()
From: Eric Dumazet @ 2017-09-11 15:22 UTC (permalink / raw)
To: liujian56
Cc: davem, kuznet, yoshfuji, edumazet, ycheng, hkchu, netdev,
weiyongjun1
In-Reply-To: <1505142785.15310.117.camel@edumazet-glaptop3.roam.corp.google.com>
On Mon, 2017-09-11 at 08:13 -0700, Eric Dumazet wrote:
> You can see we got only 3 probes, not 4.
Here is complete packetdrill test showing that code behaves as expected.
0 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
+0 setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
+0 bind(3, ..., ...) = 0
+0 listen(3, 1) = 0
+0 < S 0:0(0) win 0 <mss 1460,sackOK,nop,nop,nop,wscale 7>
+0 > S. 0:0(0) ack 1 <mss 1460,nop,nop,sackOK,nop,wscale 8>
// Client advertises a zero receive window, so we can't send.
+.1 < . 1:1(0) ack 1 win 0
+0 accept(3, ..., ...) = 4
+0 setsockopt(4, SOL_TCP, TCP_USER_TIMEOUT, [3000], 4) = 0
+0 write(4, ..., 2920) = 2920
// Window probes are scheduled just like RTOs.
+.3~+.31 > . 0:0(0) ack 1
+.6~+.62 > . 0:0(0) ack 1
+1.2~+1.24 > . 0:0(0) ack 1
// Peer opens its window too late !
+3 < . 1:1(0) ack 1 win 1000
+0 > R 1:1(0)
^ permalink raw reply
* Re: [PATCH] tcp: TCP_USER_TIMEOUT can not work in tcp_probe_timer()
From: Eric Dumazet @ 2017-09-11 15:13 UTC (permalink / raw)
To: liujian56
Cc: davem, kuznet, yoshfuji, edumazet, ycheng, hkchu, netdev,
weiyongjun1
In-Reply-To: <1505141357.15310.115.camel@edumazet-glaptop3.roam.corp.google.com>
On Mon, 2017-09-11 at 07:49 -0700, Eric Dumazet wrote:
> On Mon, 2017-09-11 at 14:27 +0800, liujian56@huawei.com wrote:
> > From: liujian <liujian56@huawei.com>
> >
> > After the tcp socket go to ESTABLISHED stat, change IP address (server
> > side),
> > then the tcp socket will go tcp_probe_timer process.
> >
> > [root@localhost net]# netstat -toe
> > Active Internet connections (w/o servers)
> > Proto Recv-Q Send-Q Local Address Foreign Address State User Inode Timer
> > tcp 0 1104 9.81.254:personal-agent 9.84.201.213:23597 ESTABLISHED root 12819 probe (4.36/0/7)
> > [root@localhost net]# cat /proc/net/tcp
> > sl local_address rem_address st tx_queue rx_queue tr tm->when retrnsmt uid timeout inode
> > 3: B1FE5109:15B3 D5C95409:5C2D 01 00000495:00000000 04:0000005E 00000000 0 7 12819 2 ffff95cdcf45a000 20 4 1 10 -1
> >
> > In my test case, tcp_write_queue_head(sk) and tcp_send_head(sk) is same
> > SKB.
> > And ((s32)(tcp_time_stamp(tp) - start_ts) >
> > jiffies_to_msecs(icsk->icsk_user_timeout))
> > always is false.
>
> Interesting.
>
> > Here use keepalive_time_elapsed(tp) to do the compare as
> > tcp_keepalive_timer do.
>
>
> But zero window probe and TCP_USER_TIMEOUT can be used without
> keepalives...
>
>
> A packetdrill test would help, I will write one.
So existing code seems to work :
# cat window-probe-without-data-user-timeout.pkt
0 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
+0 setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
+0 bind(3, ..., ...) = 0
+0 listen(3, 1) = 0
+0 < S 0:0(0) win 0 <mss 1460,sackOK,nop,nop,nop,wscale 7>
+0 > S. 0:0(0) ack 1 <mss 1460,nop,nop,sackOK,nop,wscale 8>
// Client advertises a zero receive window, so we can't send.
+.1 < . 1:1(0) ack 1 win 0
+0 accept(3, ..., ...) = 4
+0 setsockopt(4, SOL_TCP, TCP_USER_TIMEOUT, [3000], 4) = 0
+0 write(4, ..., 2920) = 2920
// Window probes are scheduled just like RTOs.
+.3~+.31 > . 0:0(0) ack 1
+.6~+.62 > . 0:0(0) ack 1
+1.2~+1.24 > . 0:0(0) ack 1
+2.4~+2.48 > . 0:0(0) ack 1
# ./packetdrill window-probe-without-data-user_timeout.pkt
08:10:39.306137 IP 192.0.2.1.58149 > 192.168.79.31.8080: Flags [S], seq 0, win 0, options [mss 1460,sackOK,nop,nop,nop,wscale 7], length 0
08:10:39.306166 IP 192.168.79.31.8080 > 192.0.2.1.58149: Flags [S.], seq 3982794529, ack 1, win 29200, options [mss 1460,nop,nop,sackOK,nop,wscale 8], length 0
08:10:39.406296 IP 192.0.2.1.58149 > 192.168.79.31.8080: Flags [.], ack 1, win 0, length 0
08:10:39.716004 IP 192.168.79.31.8080 > 192.0.2.1.58149: Flags [.], ack 1, win 115, length 0
08:10:40.327133 IP 192.168.79.31.8080 > 192.0.2.1.58149: Flags [.], ack 1, win 115, length 0
08:10:41.540243 IP 192.168.79.31.8080 > 192.0.2.1.58149: Flags [.], ack 1, win 115, length 0
You can see we got only 3 probes, not 4.
^ permalink raw reply
* Re: [PATCH] tcp: TCP_USER_TIMEOUT can not work in tcp_probe_timer()
From: Eric Dumazet @ 2017-09-11 14:49 UTC (permalink / raw)
To: liujian56
Cc: davem, kuznet, yoshfuji, edumazet, ycheng, hkchu, netdev,
weiyongjun1
In-Reply-To: <1505111262-12620-1-git-send-email-liujian56@huawei.com>
On Mon, 2017-09-11 at 14:27 +0800, liujian56@huawei.com wrote:
> From: liujian <liujian56@huawei.com>
>
> After the tcp socket go to ESTABLISHED stat, change IP address (server
> side),
> then the tcp socket will go tcp_probe_timer process.
>
> [root@localhost net]# netstat -toe
> Active Internet connections (w/o servers)
> Proto Recv-Q Send-Q Local Address Foreign Address State User Inode Timer
> tcp 0 1104 9.81.254:personal-agent 9.84.201.213:23597 ESTABLISHED root 12819 probe (4.36/0/7)
> [root@localhost net]# cat /proc/net/tcp
> sl local_address rem_address st tx_queue rx_queue tr tm->when retrnsmt uid timeout inode
> 3: B1FE5109:15B3 D5C95409:5C2D 01 00000495:00000000 04:0000005E 00000000 0 7 12819 2 ffff95cdcf45a000 20 4 1 10 -1
>
> In my test case, tcp_write_queue_head(sk) and tcp_send_head(sk) is same
> SKB.
> And ((s32)(tcp_time_stamp(tp) - start_ts) >
> jiffies_to_msecs(icsk->icsk_user_timeout))
> always is false.
Interesting.
> Here use keepalive_time_elapsed(tp) to do the compare as
> tcp_keepalive_timer do.
But zero window probe and TCP_USER_TIMEOUT can be used without
keepalives...
A packetdrill test would help, I will write one.
Thanks.
^ permalink raw reply
* [PATCH net] MAINTAINERS: Remove Yuval Mintz from maintainers list
From: aelior @ 2017-09-11 14:05 UTC (permalink / raw)
To: davem; +Cc: netdev, everest-linux-l2
From: Ariel Elior <aelior@cavium.com>
Remove Yuval from maintaining the bnx2x & qed* modules as he is no longer
working for the company. Thanks Yuval for your huge contributions and
tireless efforts over the many years and various companies.
Ariel
---
MAINTAINERS | 2 --
1 file changed, 2 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 567343b..5d24dbf 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2801,7 +2801,6 @@ S: Supported
F: drivers/scsi/bnx2i/
BROADCOM BNX2X 10 GIGABIT ETHERNET DRIVER
-M: Yuval Mintz <Yuval.Mintz@cavium.com>
M: Ariel Elior <ariel.elior@cavium.com>
M: everest-linux-l2@cavium.com
L: netdev@vger.kernel.org
@@ -10827,7 +10826,6 @@ S: Supported
F: drivers/scsi/qedi/
QLOGIC QL4xxx ETHERNET DRIVER
-M: Yuval Mintz <Yuval.Mintz@cavium.com>
M: Ariel Elior <Ariel.Elior@cavium.com>
M: everest-linux-l2@cavium.com
L: netdev@vger.kernel.org
--
2.9.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox