* [PATCH net-next] net: dsa: lan9303: Protect ALR operations with mutex
From: Egil Hjelmeland @ 2017-12-07 18:56 UTC (permalink / raw)
To: andrew, vivien.didelot, f.fainelli, netdev, linux-kernel; +Cc: Egil Hjelmeland
ALR table operations are a sequence of related register operations which
should be protected from concurrent access. The alr_cache should also be
protected. Add alr_mutex doing that.
Signed-off-by: Egil Hjelmeland <privat@egil-hjelmeland.no>
---
drivers/net/dsa/lan9303-core.c | 14 ++++++++++++--
include/linux/dsa/lan9303.h | 1 +
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c
index ea59dadefb33..c1b004fa64d9 100644
--- a/drivers/net/dsa/lan9303-core.c
+++ b/drivers/net/dsa/lan9303-core.c
@@ -583,6 +583,7 @@ static void lan9303_alr_loop(struct lan9303 *chip, alr_loop_cb_t *cb, void *ctx)
{
int i;
+ mutex_lock(&chip->alr_mutex);
lan9303_write_switch_reg(chip, LAN9303_SWE_ALR_CMD,
LAN9303_ALR_CMD_GET_FIRST);
lan9303_write_switch_reg(chip, LAN9303_SWE_ALR_CMD, 0);
@@ -606,6 +607,7 @@ static void lan9303_alr_loop(struct lan9303 *chip, alr_loop_cb_t *cb, void *ctx)
LAN9303_ALR_CMD_GET_NEXT);
lan9303_write_switch_reg(chip, LAN9303_SWE_ALR_CMD, 0);
}
+ mutex_unlock(&chip->alr_mutex);
}
static void alr_reg_to_mac(u32 dat0, u32 dat1, u8 mac[6])
@@ -694,16 +696,20 @@ static int lan9303_alr_add_port(struct lan9303 *chip, const u8 *mac, int port,
{
struct lan9303_alr_cache_entry *entr;
+ mutex_lock(&chip->alr_mutex);
entr = lan9303_alr_cache_find_mac(chip, mac);
if (!entr) { /*New entry */
entr = lan9303_alr_cache_find_free(chip);
- if (!entr)
+ if (!entr) {
+ mutex_unlock(&chip->alr_mutex);
return -ENOSPC;
+ }
ether_addr_copy(entr->mac_addr, mac);
}
entr->port_map |= BIT(port);
entr->stp_override = stp_override;
lan9303_alr_set_entry(chip, mac, entr->port_map, stp_override);
+ mutex_unlock(&chip->alr_mutex);
return 0;
}
@@ -713,15 +719,18 @@ static int lan9303_alr_del_port(struct lan9303 *chip, const u8 *mac, int port)
{
struct lan9303_alr_cache_entry *entr;
+ mutex_lock(&chip->alr_mutex);
entr = lan9303_alr_cache_find_mac(chip, mac);
if (!entr)
- return 0; /* no static entry found */
+ goto out; /* no static entry found */
entr->port_map &= ~BIT(port);
if (entr->port_map == 0) /* zero means its free again */
eth_zero_addr(entr->mac_addr);
lan9303_alr_set_entry(chip, mac, entr->port_map, entr->stp_override);
+out:
+ mutex_unlock(&chip->alr_mutex);
return 0;
}
@@ -1323,6 +1332,7 @@ int lan9303_probe(struct lan9303 *chip, struct device_node *np)
int ret;
mutex_init(&chip->indirect_mutex);
+ mutex_init(&chip->alr_mutex);
lan9303_probe_reset_gpio(chip, np);
diff --git a/include/linux/dsa/lan9303.h b/include/linux/dsa/lan9303.h
index f48a85c377de..b6514c29563f 100644
--- a/include/linux/dsa/lan9303.h
+++ b/include/linux/dsa/lan9303.h
@@ -26,6 +26,7 @@ struct lan9303 {
bool phy_addr_sel_strap;
struct dsa_switch *ds;
struct mutex indirect_mutex; /* protect indexed register access */
+ struct mutex alr_mutex; /* protect ALR access */
const struct lan9303_phy_ops *ops;
bool is_bridged; /* true if port 1 and 2 are bridged */
--
2.14.1
^ permalink raw reply related
* Re: [PATCH net 0/2] mv88e6xxx error patch fixes
From: David Miller @ 2017-12-07 18:53 UTC (permalink / raw)
To: andrew; +Cc: netdev, vivien.didelot
In-Reply-To: <1512605157-6765-1-git-send-email-andrew@lunn.ch>
From: Andrew Lunn <andrew@lunn.ch>
Date: Thu, 7 Dec 2017 01:05:55 +0100
> While trying to bring up a new PHY on a board, i exercised the error
> paths a bit, and discovered some bugs. The unwind for interrupt
> handling deadlocks, and the MDIO code hits a BUG() when a registered
> MDIO device is freed without first being unregistered.
Series applied, thanks Andrew.
^ permalink raw reply
* Re: [PATCH] net: ethernet: arc: fix error handling in emac_rockchip_probe
From: David Miller @ 2017-12-07 18:52 UTC (permalink / raw)
To: branislav; +Cc: heiko, netdev, linux-arm-kernel, linux-rockchip, linux-kernel
In-Reply-To: <20171206230738.2560-1-branislav@radocaj.org>
From: Branislav Radocaj <branislav@radocaj.org>
Date: Thu, 7 Dec 2017 00:07:38 +0100
> If clk_set_rate() fails, we should disable clk before return.
> Found by Linux Driver Verification project (linuxtesting.org).
>
> Signed-off-by: Branislav Radocaj <branislav@radocaj.org>
Ok, but this probe routine is very inconsistent in how it handles
clock rate setting failures now.
Above your changes, if the RGMII clk can't have it's rate set, we
just continue.
^ permalink raw reply
* Re: [Intel-wired-lan] [next-queue 08/10] ixgbe: process the Tx ipsec offload
From: Shannon Nelson @ 2017-12-07 18:50 UTC (permalink / raw)
To: Alexander Duyck
Cc: intel-wired-lan, Jeff Kirsher, Steffen Klassert, Sowmini Varadhan,
Netdev
In-Reply-To: <CAKgT0Udw6Ek1kw4b1LVJwfMH4xAy8-=u2j6zhdGmfJaYutWPdA@mail.gmail.com>
On 12/7/2017 9:56 AM, Alexander Duyck wrote:
You've suggested several things here, all good things to look into,
which I will do, most now, some in the near future.
Thanks!
sln
> On Wed, Dec 6, 2017 at 9:43 PM, Shannon Nelson
> <shannon.nelson@oracle.com> wrote:
>> On 12/5/2017 10:13 AM, Alexander Duyck wrote:
>>>
>>> On Mon, Dec 4, 2017 at 9:35 PM, Shannon Nelson
>>> <shannon.nelson@oracle.com> wrote:
>>>>
>>>> If the skb has a security association referenced in the skb, then
>>>> set up the Tx descriptor with the ipsec offload bits. While we're
>>>> here, we fix an oddly named field in the context descriptor struct.
>>>>
>>>> Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
>>>> ---
>>>> drivers/net/ethernet/intel/ixgbe/ixgbe.h | 10 +++-
>>>> drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c | 77
>>>> ++++++++++++++++++++++++++
>>>> drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c | 4 +-
>>>> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 38 ++++++++++---
>>>> drivers/net/ethernet/intel/ixgbe/ixgbe_type.h | 2 +-
>>>> 5 files changed, 118 insertions(+), 13 deletions(-)
>>>>
>>>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
>>>> b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
>>>> index 77f07dc..68097fe 100644
>>>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
>>>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
>>>> @@ -171,10 +171,11 @@ enum ixgbe_tx_flags {
>>>> IXGBE_TX_FLAGS_CC = 0x08,
>>>> IXGBE_TX_FLAGS_IPV4 = 0x10,
>>>> IXGBE_TX_FLAGS_CSUM = 0x20,
>>>> + IXGBE_TX_FLAGS_IPSEC = 0x40,
>>>>
>>>> /* software defined flags */
>>>> - IXGBE_TX_FLAGS_SW_VLAN = 0x40,
>>>> - IXGBE_TX_FLAGS_FCOE = 0x80,
>>>> + IXGBE_TX_FLAGS_SW_VLAN = 0x80,
>>>> + IXGBE_TX_FLAGS_FCOE = 0x100,
>>>> };
>>>>
>>>> /* VLAN info */
>>>> @@ -1012,12 +1013,17 @@ void ixgbe_init_ipsec_offload(struct
>>>> ixgbe_adapter *adapter);
>>>> void ixgbe_ipsec_rx(struct ixgbe_ring *rx_ring,
>>>> union ixgbe_adv_rx_desc *rx_desc,
>>>> struct sk_buff *skb);
>>>> +int ixgbe_ipsec_tx(struct ixgbe_ring *tx_ring, struct sk_buff *skb,
>>>> + __be16 protocol, struct ixgbe_ipsec_tx_data *itd);
>>>> void ixgbe_ipsec_restore(struct ixgbe_adapter *adapter);
>>>> #else
>>>> static inline void ixgbe_init_ipsec_offload(struct ixgbe_adapter
>>>> *adapter) { };
>>>> static inline void ixgbe_ipsec_rx(struct ixgbe_ring *rx_ring,
>>>> union ixgbe_adv_rx_desc *rx_desc,
>>>> struct sk_buff *skb) { };
>>>> +static inline int ixgbe_ipsec_tx(struct ixgbe_ring *tx_ring,
>>>> + struct sk_buff *skb, __be16 protocol,
>>>> + struct ixgbe_ipsec_tx_data *itd) {
>>>> return 0; };
>>>> static inline void ixgbe_ipsec_restore(struct ixgbe_adapter *adapter) {
>>>> };
>>>> #endif /* CONFIG_XFRM_OFFLOAD */
>>>> #endif /* _IXGBE_H_ */
>>>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
>>>> b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
>>>> index fd06d9b..2a0dd7a 100644
>>>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
>>>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
>>>> @@ -703,12 +703,89 @@ static void ixgbe_ipsec_del_sa(struct xfrm_state
>>>> *xs)
>>>> }
>>>> }
>>>>
>>>> +/**
>>>> + * ixgbe_ipsec_offload_ok - can this packet use the xfrm hw offload
>>>> + * @skb: current data packet
>>>> + * @xs: pointer to transformer state struct
>>>> + **/
>>>> +static bool ixgbe_ipsec_offload_ok(struct sk_buff *skb, struct
>>>> xfrm_state *xs)
>>>> +{
>>>> + if (xs->props.family == AF_INET) {
>>>> + /* Offload with IPv4 options is not supported yet */
>>>> + if (ip_hdr(skb)->ihl > 5)
>>>
>>>
>>> I would make this ihl != 5 instead of "> 5" since smaller values would
>>> be invalid as well.
>>
>>
>> Sure
>>
>>
>>>
>>>> + return false;
>>>> + } else {
>>>> + /* Offload with IPv6 extension headers is not support yet
>>>> */
>>>> + if (ipv6_ext_hdr(ipv6_hdr(skb)->nexthdr))
>>>> + return false;
>>>> + }
>>>> +
>>>> + return true;
>>>> +}
>>>> +
>>>> static const struct xfrmdev_ops ixgbe_xfrmdev_ops = {
>>>> .xdo_dev_state_add = ixgbe_ipsec_add_sa,
>>>> .xdo_dev_state_delete = ixgbe_ipsec_del_sa,
>>>> + .xdo_dev_offload_ok = ixgbe_ipsec_offload_ok,
>>>> };
>>>>
>>>> /**
>>>> + * ixgbe_ipsec_tx - setup Tx flags for ipsec offload
>>>> + * @tx_ring: outgoing context
>>>> + * @skb: current data packet
>>>> + * @protocol: network protocol
>>>> + * @itd: ipsec Tx data for later use in building context descriptor
>>>> + **/
>>>> +int ixgbe_ipsec_tx(struct ixgbe_ring *tx_ring, struct sk_buff *skb,
>>>> + __be16 protocol, struct ixgbe_ipsec_tx_data *itd)
>>>> +{
>>>> + struct ixgbe_adapter *adapter = netdev_priv(tx_ring->netdev);
>>>> + struct ixgbe_ipsec *ipsec = adapter->ipsec;
>>>> + struct xfrm_state *xs;
>>>> + struct tx_sa *tsa;
>>>> +
>>>> + if (!skb->sp->len) {
>>>> + netdev_err(tx_ring->netdev, "%s: no xfrm state len =
>>>> %d\n",
>>>> + __func__, skb->sp->len);
>>>> + return 0;
>>>> + }
>>>> +
>>>> + xs = xfrm_input_state(skb);
>>>> + if (!xs) {
>>>> + netdev_err(tx_ring->netdev, "%s: no xfrm_input_state() xs
>>>> = %p\n",
>>>> + __func__, xs);
>>>> + return 0;
>>>> + }
>>>> +
>>>> + itd->sa_idx = xs->xso.offload_handle - IXGBE_IPSEC_BASE_TX_INDEX;
>>>> + if (itd->sa_idx > IXGBE_IPSEC_MAX_SA_COUNT) {
>>>> + netdev_err(tx_ring->netdev, "%s: bad sa_idx=%d
>>>> handle=%lu\n",
>>>> + __func__, itd->sa_idx,
>>>> xs->xso.offload_handle);
>>>> + return 0;
>>>> + }
>>>> +
>>>> + tsa = &ipsec->tx_tbl[itd->sa_idx];
>>>> + if (!tsa->used) {
>>>> + netdev_err(tx_ring->netdev, "%s: unused sa_idx=%d\n",
>>>> + __func__, itd->sa_idx);
>>>> + return 0;
>>>> + }
>>>> +
>>>> + itd->flags = 0;
>>>> + if (xs->id.proto == IPPROTO_ESP) {
>>>> + itd->flags |= IXGBE_ADVTXD_TUCMD_IPSEC_TYPE_ESP |
>>>> + IXGBE_ADVTXD_TUCMD_L4T_TCP;
>>>
>>>
>>> Why is the TCP value being set here? This doesn't seem correct either.
>>> This implies TCP a TCP offload. It seems like this should only be
>>> setting ESP.
>>
>>
>> Honestly? Because when I was testing that, it didn't work without it. This
>> was one of the things I was going to come back to when I started working on
>> the csum and tso support.
>
> We might want to try testing with that dropped to see if we need it or
> not. I would suspect not since I would imagine this would cause bad
> things for non-TCP traffic. Also the inner L4 header shouldn't matter
> unless you are trying to offload it.
>
>>>
>>>> + if (protocol == htons(ETH_P_IP))
>>>> + itd->flags |= IXGBE_ADVTXD_TUCMD_IPV4;
>>>
>>>
>>> Does the IPsec offload need to know if the frame is v4 or v6? I'm just
>>> wondering if it does or not.
>>
>>
>> Yes, I believe this is how it knows how much header to skip to find the ESP
>> header. However, I'll test that and see if it can come out.
>
> Like I mentioned last time it might be better to have this handled in
> ixgbe_tx_csum. If it is harmless we can probably just include it
> there. We should be able to do it in the block after the no_csum
> label. I'd be curious if not doing this up until now might have other
> effects such as impacting RSS since I know the whole reason for us
> having to do the CC stuff anyway was to actually get header split to
> work correctly with PF/VF loopback packets. It wouldn't surprise me if
> setting these fields defines the packet type received on the other
> end.
>
>>> If not then this probably isn't needed.
>>> One thought on this line is you might look at moving it into
>>> ixgbe_tx_csum. If setting the bit is harmless without setting IXSM we
>>> might look at moving it into the end of ixgbe_tx_csum and just make it
>>> compare against first->protocol there.
>>
>>
>>>
>>>> + itd->trailer_len = xs->props.trailer_len;
>>>> + }
>>>> + if (tsa->encrypt)
>>>> + itd->flags |= IXGBE_ADVTXD_TUCMD_IPSEC_ENCRYPT_EN;
>>>> +
>>>> + return 1;
>>>> +}
>>>> +
>>>> +/**
>>>> * ixgbe_ipsec_rx - decode ipsec bits from Rx descriptor
>>>> * @rx_ring: receiving ring
>>>> * @rx_desc: receive data descriptor
>>>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
>>>> b/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
>>>> index f1bfae0..d7875b3 100644
>>>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
>>>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
>>>> @@ -1261,7 +1261,7 @@ void ixgbe_clear_interrupt_scheme(struct
>>>> ixgbe_adapter *adapter)
>>>> }
>>>>
>>>> void ixgbe_tx_ctxtdesc(struct ixgbe_ring *tx_ring, u32 vlan_macip_lens,
>>>> - u32 fcoe_sof_eof, u32 type_tucmd, u32
>>>> mss_l4len_idx)
>>>> + u32 fceof_saidx, u32 type_tucmd, u32
>>>> mss_l4len_idx)
>>>> {
>>>> struct ixgbe_adv_tx_context_desc *context_desc;
>>>> u16 i = tx_ring->next_to_use;
>>>> @@ -1275,7 +1275,7 @@ void ixgbe_tx_ctxtdesc(struct ixgbe_ring *tx_ring,
>>>> u32 vlan_macip_lens,
>>>> type_tucmd |= IXGBE_TXD_CMD_DEXT | IXGBE_ADVTXD_DTYP_CTXT;
>>>>
>>>> context_desc->vlan_macip_lens = cpu_to_le32(vlan_macip_lens);
>>>> - context_desc->seqnum_seed = cpu_to_le32(fcoe_sof_eof);
>>>> + context_desc->fceof_saidx = cpu_to_le32(fceof_saidx);
>>>> context_desc->type_tucmd_mlhl = cpu_to_le32(type_tucmd);
>>>> context_desc->mss_l4len_idx = cpu_to_le32(mss_l4len_idx);
>>>> }
>>>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>>> b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>>> index 60f9f2d..c857594 100644
>>>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>>> @@ -7659,9 +7659,10 @@ static void ixgbe_service_task(struct work_struct
>>>> *work)
>>>>
>>>> static int ixgbe_tso(struct ixgbe_ring *tx_ring,
>>>> struct ixgbe_tx_buffer *first,
>>>> - u8 *hdr_len)
>>>> + u8 *hdr_len,
>>>> + struct ixgbe_ipsec_tx_data *itd)
>>>> {
>>>> - u32 vlan_macip_lens, type_tucmd, mss_l4len_idx;
>>>> + u32 vlan_macip_lens, type_tucmd, mss_l4len_idx, fceof_saidx = 0;
>>>> struct sk_buff *skb = first->skb;
>>>> union {
>>>> struct iphdr *v4;
>>>> @@ -7740,7 +7741,12 @@ static int ixgbe_tso(struct ixgbe_ring *tx_ring,
>>>> vlan_macip_lens |= (ip.hdr - skb->data) <<
>>>> IXGBE_ADVTXD_MACLEN_SHIFT;
>>>> vlan_macip_lens |= first->tx_flags & IXGBE_TX_FLAGS_VLAN_MASK;
>>>>
>>>> - ixgbe_tx_ctxtdesc(tx_ring, vlan_macip_lens, 0, type_tucmd,
>>>> + if (first->tx_flags & IXGBE_TX_FLAGS_IPSEC) {
>>>> + fceof_saidx |= itd->sa_idx;
>>>> + type_tucmd |= itd->flags | itd->trailer_len;
>>>> + }
>
> So just a thought. Why bother with the TX_FLAGS_CHECK at all? It seems
> like in the case that the flag isn't set you would have itd->sa_idx
> equal to 0 anyway so it would still be the same result wouldn't it? It
> would save you from having to zero both fceof_saidx and itd->sa_idx
> since you could just pass itd->sa_idx and save yourself the extra
> variable.
>
> Also if flags and trailer_len are both being written to the same
> location why not combine them in your structure into one single 32 bit
> entry? It would allow you to essentially reduce this to one OR and you
> could just pass itd->sa_idx directly which should be a pretty
> significant savings in terms of instructions and cycles. Also you
> might want to consider bumping itd->sa_idx up to a 32b value. It will
> possibly cost you a cycle or so to convert the 16b value to a 32b
> value before writing it. If you merge the flags and trailer length you
> should have the space to spare to bump up the size.
>
>>>> +
>>>> + ixgbe_tx_ctxtdesc(tx_ring, vlan_macip_lens, fceof_saidx,
>>>> type_tucmd,
>>>> mss_l4len_idx);
>>>>
>>>> return 1;
>>>> @@ -7756,10 +7762,12 @@ static inline bool ixgbe_ipv6_csum_is_sctp(struct
>>>> sk_buff *skb)
>>>> }
>>>>
>>>> static void ixgbe_tx_csum(struct ixgbe_ring *tx_ring,
>>>> - struct ixgbe_tx_buffer *first)
>>>> + struct ixgbe_tx_buffer *first,
>>>> + struct ixgbe_ipsec_tx_data *itd)
>>>> {
>>>> struct sk_buff *skb = first->skb;
>>>> u32 vlan_macip_lens = 0;
>>>> + u32 fceof_saidx = 0;
>>>> u32 type_tucmd = 0;
>>>>
>>>> if (skb->ip_summed != CHECKSUM_PARTIAL) {
>>>> @@ -7800,7 +7808,12 @@ static void ixgbe_tx_csum(struct ixgbe_ring
>>>> *tx_ring,
>>>> vlan_macip_lens |= skb_network_offset(skb) <<
>>>> IXGBE_ADVTXD_MACLEN_SHIFT;
>>>> vlan_macip_lens |= first->tx_flags & IXGBE_TX_FLAGS_VLAN_MASK;
>>>>
>>>> - ixgbe_tx_ctxtdesc(tx_ring, vlan_macip_lens, 0, type_tucmd, 0);
>>>> + if (first->tx_flags & IXGBE_TX_FLAGS_IPSEC) {
>>>> + fceof_saidx |= itd->sa_idx;
>>>> + type_tucmd |= itd->flags | itd->trailer_len;
>>>> + }
>>>> +
>>>> + ixgbe_tx_ctxtdesc(tx_ring, vlan_macip_lens, fceof_saidx,
>>>> type_tucmd, 0);
>>>> }
>>>>
>>>> #define IXGBE_SET_FLAG(_input, _flag, _result) \
>>>> @@ -7843,11 +7856,16 @@ static void ixgbe_tx_olinfo_status(union
>>>> ixgbe_adv_tx_desc *tx_desc,
>>>> IXGBE_TX_FLAGS_CSUM,
>>>> IXGBE_ADVTXD_POPTS_TXSM);
>>>>
>>>> - /* enble IPv4 checksum for TSO */
>>>> + /* enable IPv4 checksum for TSO */
>>>> olinfo_status |= IXGBE_SET_FLAG(tx_flags,
>>>> IXGBE_TX_FLAGS_IPV4,
>>>> IXGBE_ADVTXD_POPTS_IXSM);
>>>>
>>>> + /* enable IPsec */
>>>> + olinfo_status |= IXGBE_SET_FLAG(tx_flags,
>>>> + IXGBE_TX_FLAGS_IPSEC,
>>>> + IXGBE_ADVTXD_POPTS_IPSEC);
>>>> +
>>>> /*
>>>> * Check Context must be set if Tx switch is enabled, which it
>>>> * always is for case where virtual functions are running
>>>> @@ -8306,6 +8324,7 @@ netdev_tx_t ixgbe_xmit_frame_ring(struct sk_buff
>>>> *skb,
>>>> u32 tx_flags = 0;
>>>> unsigned short f;
>>>> u16 count = TXD_USE_COUNT(skb_headlen(skb));
>>>> + struct ixgbe_ipsec_tx_data ipsec_tx = { 0 };
>>>> __be16 protocol = skb->protocol;
>>>> u8 hdr_len = 0;
>>>>
>>>> @@ -8394,6 +8413,9 @@ netdev_tx_t ixgbe_xmit_frame_ring(struct sk_buff
>>>> *skb,
>>>> }
>>>> }
>>>>
>>>> + if (skb->sp && ixgbe_ipsec_tx(tx_ring, skb, protocol, &ipsec_tx))
>>>> + tx_flags |= IXGBE_TX_FLAGS_IPSEC | IXGBE_TX_FLAGS_CC;
>>>
>>>
>>> You might just want to pull the skb->sp check into ixgbe_ipsec_tx and
>>> could pass tx_flags as a part of the first buffer. It doesn't really
>>> matter anyway as most of this will just be inlined so it will all end
>>> up a part of the same function anyway.
>>
>>
>> Since the function is defined in a different .o file, are you sure it will
>> get inlined? I put the skb->sp check here to make sure we don't do an
>> unnecessary jump.
>
> You're right. I forgot you are defining this in a different file.
>
> Still I would like to see this moved down though. Where it is at
> doesn't really flow with everything else since FCoE and this aren't
> likely to ever interact so I would rather us check for FCoE and then
> get into the IPsec logic.
>
>>>
>>> Also I would move this down so that it is handled after the fields in
>>> the first buffer_info structure are set. Then this can ll just fall
>>> inline with the TSO block and get handled there.
>>>
>>>> +
>>>> /* record initial flags and protocol */
>>>> first->tx_flags = tx_flags;
>>>> first->protocol = protocol;
>>>> @@ -8410,11 +8432,11 @@ netdev_tx_t ixgbe_xmit_frame_ring(struct sk_buff
>>>> *skb,
>>>> }
>>>>
>>>> #endif /* IXGBE_FCOE */
>>>
>>>
>>> So if you move the function down here it will help to avoid any other
>>> complication. In addition you could follow the same logic that we do
>>> for ixgbe_tso/fso so you could drop the frame instead of transmitting
>>> it if it is requesting a bad offload.
>>
>>
>> Sure
>>
>> sln
>>
>>
>>>
>>>> - tso = ixgbe_tso(tx_ring, first, &hdr_len);
>>>> + tso = ixgbe_tso(tx_ring, first, &hdr_len, &ipsec_tx);
>>>> if (tso < 0)
>>>> goto out_drop;
>>>> else if (!tso)
>>>> - ixgbe_tx_csum(tx_ring, first);
>>>> + ixgbe_tx_csum(tx_ring, first, &ipsec_tx);
>>>>
>>>> /* add the ATR filter if ATR is on */
>>>> if (test_bit(__IXGBE_TX_FDIR_INIT_DONE, &tx_ring->state))
>>>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
>>>> b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
>>>> index 3df0763..0ac725fa 100644
>>>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
>>>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
>>>> @@ -2856,7 +2856,7 @@ union ixgbe_adv_rx_desc {
>>>> /* Context descriptors */
>>>> struct ixgbe_adv_tx_context_desc {
>>>> __le32 vlan_macip_lens;
>>>> - __le32 seqnum_seed;
>>>> + __le32 fceof_saidx;
>>>> __le32 type_tucmd_mlhl;
>>>> __le32 mss_l4len_idx;
>>>> };
>>>> --
>>>> 2.7.4
>>>>
>>>> _______________________________________________
>>>> Intel-wired-lan mailing list
>>>> Intel-wired-lan@osuosl.org
>>>> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply
* Re: [Bloat] Linux network is damn fast, need more use XDP (Was: DC behaviors today)
From: Matthias Tafelmeier @ 2017-12-07 18:50 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Dave Taht, netdev@vger.kernel.org, Joel Wirāmu Pauling,
David Ahern, Tariq Toukan, Björn Töpel
In-Reply-To: <20171207093343.071083ff@redhat.com>
[-- Attachment #1.1.1: Type: text/plain, Size: 3284 bytes --]
That's the discussion I meant:
https://www.youtube.com/watch?v=vsjxgOpv1n8
Manifold excuses should I've turned your words in any respect. On top,
haven't rewatched it, only putting it here for completeness' sake.
> You are mis-quoting me. I have not recommended tremendously increasing
> the RX/TX rings(queues) numbers. Actually, we should likely decrease
> number of RX-rings, per recommendation of Eric Dumazet[1], to increase
> the chance of packet aggregation/bulking during NAPI-loop. And use
> something like CPUMAP[2] to re-distribute load on CPUs.
>
> [1] https://www.netdevconf.org/2.1/papers/BusyPollingNextGen.pdf
> [2] https://git.kernel.org/torvalds/c/452606d6c9cd
Well certainly so for throughut optimizations: allow me to defensively
qualify by saying, screwing down latency still quite depends on scaling
out rings, at least that's what experience tells me for the NAPI based
approach. Should hold symmetrically for Busy-Polling, though, am only
theoriticising there.
>
> You might have heard/seen me talk about increasing the ring queue size.
> that is the frames/pages available per RX-ring queue[3][4]. I generally
> don't recommend increasing that too much, as it hurts cache-usage. The
> real reason it sometimes helps to increase the RX-ring size on the
> Intel based NICs is because they intermix page-recycling into their
> RX-ring, which I now added a counter for when it fails[5].
>
> [3] http://netoptimizer.blogspot.dk/2014/10/unlocked-10gbps-tx-wirespeed-smallest.html
> [4] http://netoptimizer.blogspot.dk/2014/06/pktgen-for-network-overload-testing.html
> [5] https://git.kernel.org/torvalds/c/86e23494222f3
>
Presumingly, touching the length shoulb be obsolete ever since DQL,
respectively BQL, anyways.
>> Further, that there are hardly
>> any limits to the number other than FPGA magic/physical HW - up to
>> millions is viable was coined back then. May I ask were this ended up?
>> Wouldn't that be key for massive parallelization either - With having a
>> queue(producer), a CPU (consumer) - vice versa - per flow at the
>> extreme? Did this end up in this SMART-NIC thingummy? The latter is
>> rather trageted at XDP, no?
> I do have future plans for (wanting drivers to support) dynamically
> adding more RX-TX-queue-pairs. The general idea is to have NIC HW to
> filter packets per application into specific NIC queue number, which
> can be mapped directly into an application (and I want a queue-pair to
> allow the app to TX also).
> I actually imagine that we can do the application steering via
> XDP_REDIRECT. And by having application register user-pages, like
> AF_PACKET-V4, we can achieve zero-copy into userspace from XDP.
I understand, working on a sort of in kernel 'virtual' TX-RX-Ring-Pairs
per flow/application.
> A
> subtle trick here is that zero-copy only occurs if the RX-queue number
> match (XDP operating at driver ring level could know), meaning that NIC
> HW filter setup could happen async (but premapping userspace pages
> still have to happen upfront, before starting app/socket).
>
I see, a more sophisticated, flexible RPS then. That was overdue.
Very much appreciated, thanks!
--
Besten Gruß
Matthias Tafelmeier
[-- Attachment #1.1.2: 0x8ADF343B.asc --]
[-- Type: application/pgp-keys, Size: 4806 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 538 bytes --]
^ permalink raw reply
* Re: [PATCH net-next v3] net: dsa: Allow compiling out legacy support
From: David Miller @ 2017-12-07 18:45 UTC (permalink / raw)
To: f.fainelli; +Cc: netdev, andrew, vivien.didelot, linux-kernel
In-Reply-To: <20171206230333.7260-1-f.fainelli@gmail.com>
From: Florian Fainelli <f.fainelli@gmail.com>
Date: Wed, 6 Dec 2017 15:03:33 -0800
> Introduce a configuration option: CONFIG_NET_DSA_LEGACY allowing to compile out
> support for the old platform device and Device Tree binding registration.
> Support for these configurations is scheduled to be removed in 4.17.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH net-next v2 1/5] net: Introduce NETIF_F_GRO_HW.
From: Michael Chan @ 2017-12-07 18:44 UTC (permalink / raw)
To: Alexander Duyck
Cc: David Miller, Netdev, Andrew Gospodarek, Ariel Elior,
everest-linux-l2
In-Reply-To: <CAKgT0UcKOQWxu32cRnhBsu9V6ak-DAsMhWc_bqJ2Jm=MT=ww8A@mail.gmail.com>
On Thu, Dec 7, 2017 at 10:13 AM, Alexander Duyck
<alexander.duyck@gmail.com> wrote:
> On Thu, Dec 7, 2017 at 12:03 AM, Michael Chan <michael.chan@broadcom.com> wrote:
>> @@ -7405,6 +7405,23 @@ static netdev_features_t netdev_fix_features(struct net_device *dev,
>> features &= ~dev->gso_partial_features;
>> }
>>
>> + if (features & NETIF_F_GRO_HW) {
>> + /* Hardware GRO depends on GRO and RXCSUM. */
>> + if (!(features & NETIF_F_GRO)) {
>> + netdev_dbg(dev, "Dropping NETIF_F_GSO_HW since no GRO feature.\n");
>> + features &= ~NETIF_F_GRO_HW;
>> + }
>
> I'm not sure I agree with this part. The hardware offload should not
> be dependent on the software offload.
As I said before, GRO_HW is basically hardware accelerated GRO. To me,
it makes perfect sense that GRO_HW depends on GRO.
> If you are concerned with the
> XDP case and such you should probably just look at handling it more
> like how TSO is handled with a group of flags that aggregate GRO, LRO,
> and GRO_HW into a group of features that must be disabled to support
> XDP.
>
>> + if (!(features & NETIF_F_RXCSUM)) {
>> + netdev_dbg(dev, "Dropping NETIF_F_GSO_HW since no RXCSUM feature.\n");
>> + features &= ~NETIF_F_GRO_HW;
>> + }
>> + /* Hardware GRO and LRO are mutually exclusive. */
>> + if (features & NETIF_F_LRO) {
>> + netdev_dbg(dev, "Dropping NETIF_F_LRO since GRO_HW is set.\n");
>> + features &= ~NETIF_F_LRO;
>> + }
>
> This is going to be problematic. What if you bond one interface that
> has LRO and one that has GRO_HW? It seems like this is going to cause
> the bond interface to lie and say LRO isn't there when it actually is,
> or worse yet it will force features off when they shouldn't be.
This is just dropping incompatible features similar to how other
incompatible feature flags are dropped in netdev_fix_features().
Hardware cannot aggregate in both LRO and GRO_HW modes at the same
time. It's one or the other.
LRO and GRO_HW are different. We may never agree on this but they are
different. If a bond needs to disable LRO, it will be propagated to
all slaves. But each slave can have GRO enabled. If GRO is enabled,
GRO_HW, being a subset of GRO, can be enabled.
^ permalink raw reply
* Re: [PATCH] net: mvmdio: disable/unprepare clocks in EPROBE_DEFER case
From: David Miller @ 2017-12-07 18:41 UTC (permalink / raw)
To: Tobias.Jordan; +Cc: netdev, linux-kernel, ldv-project
In-Reply-To: <20171206142323.6hbm54leg2maanbj@agrajag.zerfleddert.de>
From: Tobias Jordan <Tobias.Jordan@elektrobit.com>
Date: Wed, 6 Dec 2017 15:23:23 +0100
> add appropriate calls to clk_disable_unprepare() by jumping to out_mdio
> in case orion_mdio_probe() returns -EPROBE_DEFER.
>
> Found by Linux Driver Verification project (linuxtesting.org).
>
> Fixes: 3d604da1e954 net: mvmdio: get and enable optional clock
> Signed-off-by: Tobias Jordan <Tobias.Jordan@elektrobit.com>
Applied, but please in the future format your Fixes: tag properly.
You need to put the commit message header line inside of parenthesis
and double quotes, like this:
Fixes: 3d604da1e954 ("net: mvmdio: get and enable optional clock")
^ permalink raw reply
* Re: [PATCH net-next v3 0/2] net: thunderx: add support for PTP clock
From: David Miller @ 2017-12-07 18:39 UTC (permalink / raw)
To: aleksey.makarov
Cc: netdev, linux-arm-kernel, linux-kernel, Sunil.Goutham, rad, rric,
ddaney
In-Reply-To: <20171206133100.26436-1-aleksey.makarov@cavium.com>
From: Aleksey Makarov <aleksey.makarov@cavium.com>
Date: Wed, 6 Dec 2017 16:30:56 +0300
> This series adds support for IEEE 1588 Precision Time Protocol
> to Cavium ethernet driver.
Series applied, thank you.
^ permalink raw reply
* [PATCH v2 net-next 4/4] bpftool: implement cgroup bpf operations
From: Roman Gushchin @ 2017-12-07 18:39 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, kernel-team, ast, daniel, jakub.kicinski, kafai,
guro, Quentin Monnet, David Ahern
In-Reply-To: <20171207183909.16240-1-guro@fb.com>
This patch adds basic cgroup bpf operations to bpftool:
cgroup list, attach and detach commands.
Usage is described in the corresponding man pages,
and examples are provided.
Syntax:
$ bpftool cgroup list CGROUP
$ bpftool cgroup attach CGROUP ATTACH_TYPE PROG [ATTACH_FLAGS]
$ bpftool cgroup detach CGROUP ATTACH_TYPE PROG
Signed-off-by: Roman Gushchin <guro@fb.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Jakub Kicinski <jakub.kicinski@netronome.com>
Cc: Martin KaFai Lau <kafai@fb.com>
Cc: Quentin Monnet <quentin.monnet@netronome.com>
Cc: David Ahern <dsahern@gmail.com>
---
tools/bpf/bpftool/Documentation/bpftool-cgroup.rst | 92 +++++++
tools/bpf/bpftool/Documentation/bpftool-map.rst | 2 +-
tools/bpf/bpftool/Documentation/bpftool-prog.rst | 2 +-
tools/bpf/bpftool/Documentation/bpftool.rst | 6 +-
tools/bpf/bpftool/cgroup.c | 305 +++++++++++++++++++++
tools/bpf/bpftool/main.c | 3 +-
tools/bpf/bpftool/main.h | 1 +
7 files changed, 406 insertions(+), 5 deletions(-)
create mode 100644 tools/bpf/bpftool/Documentation/bpftool-cgroup.rst
create mode 100644 tools/bpf/bpftool/cgroup.c
diff --git a/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst b/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst
new file mode 100644
index 000000000000..61ded613aee1
--- /dev/null
+++ b/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst
@@ -0,0 +1,92 @@
+================
+bpftool-cgroup
+================
+-------------------------------------------------------------------------------
+tool for inspection and simple manipulation of eBPF progs
+-------------------------------------------------------------------------------
+
+:Manual section: 8
+
+SYNOPSIS
+========
+
+ **bpftool** [*OPTIONS*] **cgroup** *COMMAND*
+
+ *OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] | { **-f** | **--bpffs** } }
+
+ *COMMANDS* :=
+ { **list** | **attach** | **detach** | **help** }
+
+MAP COMMANDS
+=============
+
+| **bpftool** **cgroup list** *CGROUP*
+| **bpftool** **cgroup attach** *CGROUP* *ATTACH_TYPE* *PROG* [*ATTACH_FLAGS*]
+| **bpftool** **cgroup detach** *CGROUP* *ATTACH_TYPE* *PROG*
+| **bpftool** **cgroup help**
+|
+| *PROG* := { **id** *PROG_ID* | **pinned** *FILE* | **tag** *PROG_TAG* }
+
+DESCRIPTION
+===========
+ **bpftool cgroup list** *CGROUP*
+ List all programs attached to the cgroup *CGROUP*.
+
+ Output will start with program ID followed by attach type,
+ attach flags and program name.
+
+ **bpftool cgroup attach** *CGROUP* *ATTACH_TYPE* *PROG* [*ATTACH_FLAGS*]
+ Attach program *PROG* to the cgroup *CGROUP* with attach type
+ *ATTACH_TYPE* and optional *ATTACH_FLAGS*.
+
+ **bpftool cgroup detach** *CGROUP* *ATTACH_TYPE* *PROG*
+ Detach *PROG* from the cgroup *CGROUP* and attach type
+ *ATTACH_TYPE*.
+
+ **bpftool prog help**
+ Print short help message.
+
+OPTIONS
+=======
+ -h, --help
+ Print short generic help message (similar to **bpftool help**).
+
+ -v, --version
+ Print version number (similar to **bpftool version**).
+
+ -j, --json
+ Generate JSON output. For commands that cannot produce JSON, this
+ option has no effect.
+
+ -p, --pretty
+ Generate human-readable JSON output. Implies **-j**.
+
+ -f, --bpffs
+ Show file names of pinned programs.
+
+EXAMPLES
+========
+|
+| **# mount -t bpf none /sys/fs/bpf/**
+| **# mkdir /sys/fs/cgroup/test.slice**
+| **# bpftool prog load ./device_cgroup.o /sys/fs/bpf/prog**
+| **# bpftool cgroup attach /sys/fs/cgroup/test.slice/ device id 1 allow_multi**
+
+**# bpftool cgroup list /sys/fs/cgroup/test.slice/**
+
+::
+
+ ID AttachType AttachFlags Name
+ 1 device allow_multi bpf_prog1
+
+|
+| **# bpftool cgroup detach /sys/fs/cgroup/test.slice/ device id 1**
+| **# bpftool cgroup list /sys/fs/cgroup/test.slice/**
+
+::
+
+ ID AttachType AttachFlags Name
+
+SEE ALSO
+========
+ **bpftool**\ (8), **bpftool-prog**\ (8), **bpftool-map**\ (8)
diff --git a/tools/bpf/bpftool/Documentation/bpftool-map.rst b/tools/bpf/bpftool/Documentation/bpftool-map.rst
index 9f51a268eb06..421cabc417e6 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-map.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-map.rst
@@ -128,4 +128,4 @@ EXAMPLES
SEE ALSO
========
- **bpftool**\ (8), **bpftool-prog**\ (8)
+ **bpftool**\ (8), **bpftool-prog**\ (8), **bpftool-cgroup**\ (8)
diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
index 827b415f8ab6..61229a1779a3 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
@@ -155,4 +155,4 @@ EXAMPLES
SEE ALSO
========
- **bpftool**\ (8), **bpftool-map**\ (8)
+ **bpftool**\ (8), **bpftool-map**\ (8), **bpftool-cgroup**\ (8)
diff --git a/tools/bpf/bpftool/Documentation/bpftool.rst b/tools/bpf/bpftool/Documentation/bpftool.rst
index f547a0c0aa34..6732a5a617e4 100644
--- a/tools/bpf/bpftool/Documentation/bpftool.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool.rst
@@ -16,7 +16,7 @@ SYNOPSIS
**bpftool** **version**
- *OBJECT* := { **map** | **program** }
+ *OBJECT* := { **map** | **program** | **cgroup** }
*OPTIONS* := { { **-V** | **--version** } | { **-h** | **--help** }
| { **-j** | **--json** } [{ **-p** | **--pretty** }] }
@@ -28,6 +28,8 @@ SYNOPSIS
*PROG-COMMANDS* := { **show** | **dump jited** | **dump xlated** | **pin**
| **load** | **help** }
+ *CGROUP-COMMANDS* := { **list** | **attach** | **detach** | **help** }
+
DESCRIPTION
===========
*bpftool* allows for inspection and simple modification of BPF objects
@@ -53,4 +55,4 @@ OPTIONS
SEE ALSO
========
- **bpftool-map**\ (8), **bpftool-prog**\ (8)
+ **bpftool-map**\ (8), **bpftool-prog**\ (8), **bpftool-cgroup**\ (8)
diff --git a/tools/bpf/bpftool/cgroup.c b/tools/bpf/bpftool/cgroup.c
new file mode 100644
index 000000000000..88d67f74313f
--- /dev/null
+++ b/tools/bpf/bpftool/cgroup.c
@@ -0,0 +1,305 @@
+/*
+ * Copyright (C) 2017 Facebook
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ *
+ * Author: Roman Gushchin <guro@fb.com>
+ */
+
+#include <fcntl.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <unistd.h>
+
+#include <bpf.h>
+
+#include "main.h"
+
+static const char * const attach_type_strings[] = {
+ [BPF_CGROUP_INET_INGRESS] = "ingress",
+ [BPF_CGROUP_INET_EGRESS] = "egress",
+ [BPF_CGROUP_INET_SOCK_CREATE] = "sock_create",
+ [BPF_CGROUP_SOCK_OPS] = "sock_ops",
+ [BPF_CGROUP_DEVICE] = "device",
+ [__MAX_BPF_ATTACH_TYPE] = NULL,
+};
+
+static enum bpf_attach_type parse_attach_type(const char *str)
+{
+ enum bpf_attach_type type;
+
+ for (type = 0; type < __MAX_BPF_ATTACH_TYPE; type++) {
+ if (attach_type_strings[type] &&
+ strcmp(str, attach_type_strings[type]) == 0)
+ return type;
+ }
+
+ return __MAX_BPF_ATTACH_TYPE;
+}
+
+static int list_bpf_prog(int id, const char *attach_type_str,
+ const char *attach_flags_str)
+{
+ struct bpf_prog_info info = {};
+ __u32 info_len = sizeof(info);
+ int prog_fd;
+
+ prog_fd = bpf_prog_get_fd_by_id(id);
+ if (prog_fd < 0)
+ return -1;
+
+ if (bpf_obj_get_info_by_fd(prog_fd, &info, &info_len)) {
+ close(prog_fd);
+ return -1;
+ }
+
+ if (json_output) {
+ jsonw_start_object(json_wtr);
+ jsonw_uint_field(json_wtr, "id", info.id);
+ jsonw_string_field(json_wtr, "attach_type",
+ attach_type_str);
+ jsonw_string_field(json_wtr, "attach_flags",
+ attach_flags_str);
+ jsonw_string_field(json_wtr, "name", info.name);
+ jsonw_end_object(json_wtr);
+ } else {
+ printf("%-8u %-15s %-15s %-15s\n", info.id,
+ attach_type_str,
+ attach_flags_str,
+ info.name);
+ }
+
+ close(prog_fd);
+ return 0;
+}
+
+static int list_attached_bpf_progs(int cgroup_fd, enum bpf_attach_type type)
+{
+ __u32 attach_flags;
+ __u32 prog_ids[1024] = {0};
+ __u32 prog_cnt, iter;
+ char *attach_flags_str;
+ int ret;
+
+ prog_cnt = ARRAY_SIZE(prog_ids);
+ ret = bpf_prog_query(cgroup_fd, type, 0, &attach_flags, prog_ids,
+ &prog_cnt);
+ if (ret)
+ return ret;
+
+ if (prog_cnt == 0)
+ return 0;
+
+ switch (attach_flags) {
+ case BPF_F_ALLOW_MULTI:
+ attach_flags_str = "allow_multi";
+ break;
+ case BPF_F_ALLOW_OVERRIDE:
+ attach_flags_str = "allow_override";
+ break;
+ case 0:
+ attach_flags_str = "";
+ break;
+ default:
+ attach_flags_str = "unknown";
+ }
+
+ for (iter = 0; iter < prog_cnt; iter++)
+ list_bpf_prog(prog_ids[iter], attach_type_strings[type],
+ attach_flags_str);
+
+ return 0;
+}
+
+static int do_list(int argc, char **argv)
+{
+ enum bpf_attach_type type;
+ int cgroup_fd;
+ int ret = -1;
+
+ if (argc < 1) {
+ p_err("too few parameters for cgroup list\n");
+ goto exit;
+ } else if (argc > 1) {
+ p_err("too many parameters for cgroup list\n");
+ goto exit;
+ }
+
+ cgroup_fd = open(argv[0], O_RDONLY);
+ if (cgroup_fd < 0) {
+ p_err("can't open cgroup %s\n", argv[1]);
+ goto exit;
+ }
+
+ if (json_output)
+ jsonw_start_array(json_wtr);
+ else
+ printf("%-8s %-15s %-15s %-15s\n", "ID", "AttachType",
+ "AttachFlags", "Name");
+
+ for (type = 0; type < __MAX_BPF_ATTACH_TYPE; type++) {
+ /*
+ * Not all attach types may be supported, so it's expected,
+ * that some requests will fail.
+ * If we were able to get the list for at least one
+ * attach type, let's return 0.
+ */
+ if (list_attached_bpf_progs(cgroup_fd, type) == 0)
+ ret = 0;
+ }
+
+ if (json_output)
+ jsonw_end_array(json_wtr);
+
+ close(cgroup_fd);
+exit:
+ return ret;
+}
+
+static int do_attach(int argc, char **argv)
+{
+ int cgroup_fd, prog_fd;
+ enum bpf_attach_type attach_type;
+ int attach_flags = 0;
+ int i;
+ int ret = -1;
+
+ if (argc < 4) {
+ p_err("too few parameters for cgroup attach\n");
+ goto exit;
+ }
+
+ cgroup_fd = open(argv[0], O_RDONLY);
+ if (cgroup_fd < 0) {
+ p_err("can't open cgroup %s\n", argv[1]);
+ goto exit;
+ }
+
+ attach_type = parse_attach_type(argv[1]);
+ if (attach_type == __MAX_BPF_ATTACH_TYPE) {
+ p_err("invalid attach type\n");
+ goto exit_cgroup;
+ }
+
+ argc -= 2;
+ argv = &argv[2];
+ prog_fd = prog_parse_fd(&argc, &argv);
+ if (prog_fd < 0)
+ goto exit_cgroup;
+
+ for (i = 0; i < argc; i++) {
+ if (strcmp(argv[i], "allow_multi") == 0) {
+ attach_flags |= BPF_F_ALLOW_MULTI;
+ } else if (strcmp(argv[i], "allow_override") == 0) {
+ attach_flags |= BPF_F_ALLOW_OVERRIDE;
+ } else {
+ p_err("unknown option: %s\n", argv[i]);
+ goto exit_cgroup;
+ }
+ }
+
+ if (bpf_prog_attach(prog_fd, cgroup_fd, attach_type, attach_flags)) {
+ p_err("failed to attach program");
+ goto exit_prog;
+ }
+
+ if (json_output)
+ jsonw_null(json_wtr);
+
+ ret = 0;
+
+exit_prog:
+ close(prog_fd);
+exit_cgroup:
+ close(cgroup_fd);
+exit:
+ return ret;
+}
+
+static int do_detach(int argc, char **argv)
+{
+ int prog_fd, cgroup_fd;
+ enum bpf_attach_type attach_type;
+ int ret = -1;
+
+ if (argc < 4) {
+ p_err("too few parameters for cgroup detach\n");
+ goto exit;
+ }
+
+ cgroup_fd = open(argv[0], O_RDONLY);
+ if (cgroup_fd < 0) {
+ p_err("can't open cgroup %s\n", argv[1]);
+ goto exit;
+ }
+
+ attach_type = parse_attach_type(argv[1]);
+ if (attach_type == __MAX_BPF_ATTACH_TYPE) {
+ p_err("invalid attach type");
+ goto exit_cgroup;
+ }
+
+ argc -= 2;
+ argv = &argv[2];
+ prog_fd = prog_parse_fd(&argc, &argv);
+ if (prog_fd < 0)
+ goto exit_cgroup;
+
+ if (bpf_prog_detach2(prog_fd, cgroup_fd, attach_type)) {
+ p_err("failed to attach program");
+ goto exit_prog;
+ }
+
+ if (json_output)
+ jsonw_null(json_wtr);
+
+ ret = 0;
+
+exit_prog:
+ close(prog_fd);
+exit_cgroup:
+ close(cgroup_fd);
+exit:
+ return ret;
+}
+
+static int do_help(int argc, char **argv)
+{
+ if (json_output) {
+ jsonw_null(json_wtr);
+ return 0;
+ }
+
+ fprintf(stderr,
+ "Usage: %s %s list CGROUP\n"
+ " %s %s attach CGROUP TYPE PROG [ATTACH_FLAGS]\n"
+ " %s %s detach CGROUP TYPE PROG\n"
+ " %s %s help\n"
+ "\n"
+ " ATTACH_FLAGS := { allow_multi | allow_override }"
+ " " HELP_SPEC_PROGRAM "\n"
+ " " HELP_SPEC_OPTIONS "\n"
+ "",
+ bin_name, argv[-2], bin_name, argv[-2],
+ bin_name, argv[-2], bin_name, argv[-2]);
+
+ return 0;
+}
+
+static const struct cmd cmds[] = {
+ { "list", do_list },
+ { "attach", do_attach },
+ { "detach", do_detach },
+ { "help", do_help },
+ { 0 }
+};
+
+int do_cgroup(int argc, char **argv)
+{
+ return cmd_select(cmds, argc, argv, do_help);
+}
diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
index d294bc8168be..ecd53ccf1239 100644
--- a/tools/bpf/bpftool/main.c
+++ b/tools/bpf/bpftool/main.c
@@ -85,7 +85,7 @@ static int do_help(int argc, char **argv)
" %s batch file FILE\n"
" %s version\n"
"\n"
- " OBJECT := { prog | map }\n"
+ " OBJECT := { prog | map | cgroup }\n"
" " HELP_SPEC_OPTIONS "\n"
"",
bin_name, bin_name, bin_name);
@@ -173,6 +173,7 @@ static const struct cmd cmds[] = {
{ "batch", do_batch },
{ "prog", do_prog },
{ "map", do_map },
+ { "cgroup", do_cgroup },
{ "version", do_version },
{ 0 }
};
diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
index bec1ccbb49c7..8f6d3cac0347 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -115,6 +115,7 @@ int do_pin_fd(int fd, const char *name);
int do_prog(int argc, char **arg);
int do_map(int argc, char **arg);
+int do_cgroup(int argc, char **arg);
int prog_parse_fd(int *argc, char ***argv);
--
2.14.3
^ permalink raw reply related
* [PATCH v2 net-next 3/4] bpftool: implement prog load command
From: Roman Gushchin @ 2017-12-07 18:39 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, kernel-team, ast, daniel, jakub.kicinski, kafai,
guro, Quentin Monnet, David Ahern
In-Reply-To: <20171207183909.16240-1-guro@fb.com>
Add the prog load command to load a bpf program from a specified
binary file and pin it to bpffs.
Usage description and examples are given in the corresponding man
page.
Syntax:
$ bpftool prog load SOURCE_FILE FILE
FILE is a non-existing file on bpffs.
Signed-off-by: Roman Gushchin <guro@fb.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Jakub Kicinski <jakub.kicinski@netronome.com>
Cc: Martin KaFai Lau <kafai@fb.com>
Cc: Quentin Monnet <quentin.monnet@netronome.com>
Cc: David Ahern <dsahern@gmail.com>
---
tools/bpf/bpftool/Documentation/bpftool-prog.rst | 10 +++-
tools/bpf/bpftool/Documentation/bpftool.rst | 2 +-
tools/bpf/bpftool/common.c | 71 +++++++++++++-----------
tools/bpf/bpftool/main.h | 1 +
tools/bpf/bpftool/prog.c | 31 ++++++++++-
5 files changed, 81 insertions(+), 34 deletions(-)
diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
index 36e8d1c3c40d..827b415f8ab6 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
@@ -15,7 +15,7 @@ SYNOPSIS
*OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] | { **-f** | **--bpffs** } }
*COMMANDS* :=
- { **show** | **dump xlated** | **dump jited** | **pin** | **help** }
+ { **show** | **dump xlated** | **dump jited** | **pin** | **load** | **help** }
MAP COMMANDS
=============
@@ -24,6 +24,7 @@ MAP COMMANDS
| **bpftool** **prog dump xlated** *PROG* [{**file** *FILE* | **opcodes**}]
| **bpftool** **prog dump jited** *PROG* [{**file** *FILE* | **opcodes**}]
| **bpftool** **prog pin** *PROG* *FILE*
+| **bpftool** **prog load** *SRC* *FILE*
| **bpftool** **prog help**
|
| *PROG* := { **id** *PROG_ID* | **pinned** *FILE* | **tag** *PROG_TAG* }
@@ -57,6 +58,11 @@ DESCRIPTION
Note: *FILE* must be located in *bpffs* mount.
+ **bpftool prog load** *SRC* *FILE*
+ Load bpf program from binary *SRC* and pin as *FILE*.
+
+ Note: *FILE* must be located in *bpffs* mount.
+
**bpftool prog help**
Print short help message.
@@ -126,8 +132,10 @@ EXAMPLES
|
| **# mount -t bpf none /sys/fs/bpf/**
| **# bpftool prog pin id 10 /sys/fs/bpf/prog**
+| **# bpftool prog load ./my_prog.o /sys/fs/bpf/prog2**
| **# ls -l /sys/fs/bpf/**
| -rw------- 1 root root 0 Jul 22 01:43 prog
+| -rw------- 1 root root 0 Dec 07 17:23 prog2
**# bpftool prog dum jited pinned /sys/fs/bpf/prog opcodes**
diff --git a/tools/bpf/bpftool/Documentation/bpftool.rst b/tools/bpf/bpftool/Documentation/bpftool.rst
index 926c03d5a8da..f547a0c0aa34 100644
--- a/tools/bpf/bpftool/Documentation/bpftool.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool.rst
@@ -26,7 +26,7 @@ SYNOPSIS
| **pin** | **help** }
*PROG-COMMANDS* := { **show** | **dump jited** | **dump xlated** | **pin**
- | **help** }
+ | **load** | **help** }
DESCRIPTION
===========
diff --git a/tools/bpf/bpftool/common.c b/tools/bpf/bpftool/common.c
index 2bd3b280e6dd..b62c94e3997a 100644
--- a/tools/bpf/bpftool/common.c
+++ b/tools/bpf/bpftool/common.c
@@ -163,13 +163,49 @@ int open_obj_pinned_any(char *path, enum bpf_obj_type exp_type)
return fd;
}
-int do_pin_any(int argc, char **argv, int (*get_fd_by_id)(__u32))
+int do_pin_fd(int fd, const char *name)
{
char err_str[ERR_MAX_LEN];
- unsigned int id;
- char *endptr;
char *file;
char *dir;
+ int err = 0;
+
+ err = bpf_obj_pin(fd, name);
+ if (!err)
+ goto out;
+
+ file = malloc(strlen(name) + 1);
+ strcpy(file, name);
+ dir = dirname(file);
+
+ if (errno != EPERM || is_bpffs(dir)) {
+ p_err("can't pin the object (%s): %s", name, strerror(errno));
+ goto out_free;
+ }
+
+ /* Attempt to mount bpffs, then retry pinning. */
+ err = mnt_bpffs(dir, err_str, ERR_MAX_LEN);
+ if (!err) {
+ err = bpf_obj_pin(fd, name);
+ if (err)
+ p_err("can't pin the object (%s): %s", name,
+ strerror(errno));
+ } else {
+ err_str[ERR_MAX_LEN - 1] = '\0';
+ p_err("can't mount BPF file system to pin the object (%s): %s",
+ name, err_str);
+ }
+
+out_free:
+ free(file);
+out:
+ return err;
+}
+
+int do_pin_any(int argc, char **argv, int (*get_fd_by_id)(__u32))
+{
+ unsigned int id;
+ char *endptr;
int err;
int fd;
@@ -195,35 +231,8 @@ int do_pin_any(int argc, char **argv, int (*get_fd_by_id)(__u32))
return -1;
}
- err = bpf_obj_pin(fd, *argv);
- if (!err)
- goto out_close;
-
- file = malloc(strlen(*argv) + 1);
- strcpy(file, *argv);
- dir = dirname(file);
-
- if (errno != EPERM || is_bpffs(dir)) {
- p_err("can't pin the object (%s): %s", *argv, strerror(errno));
- goto out_free;
- }
+ err = do_pin_fd(fd, *argv);
- /* Attempt to mount bpffs, then retry pinning. */
- err = mnt_bpffs(dir, err_str, ERR_MAX_LEN);
- if (!err) {
- err = bpf_obj_pin(fd, *argv);
- if (err)
- p_err("can't pin the object (%s): %s", *argv,
- strerror(errno));
- } else {
- err_str[ERR_MAX_LEN - 1] = '\0';
- p_err("can't mount BPF file system to pin the object (%s): %s",
- *argv, err_str);
- }
-
-out_free:
- free(file);
-out_close:
close(fd);
return err;
}
diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
index bff330b49791..bec1ccbb49c7 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -111,6 +111,7 @@ char *get_fdinfo(int fd, const char *key);
int open_obj_pinned(char *path);
int open_obj_pinned_any(char *path, enum bpf_obj_type exp_type);
int do_pin_any(int argc, char **argv, int (*get_fd_by_id)(__u32));
+int do_pin_fd(int fd, const char *name);
int do_prog(int argc, char **arg);
int do_map(int argc, char **arg);
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index ad619b96c276..bac5d81e2ff0 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -45,6 +45,7 @@
#include <sys/stat.h>
#include <bpf.h>
+#include <libbpf.h>
#include "main.h"
#include "disasm.h"
@@ -635,6 +636,32 @@ static int do_pin(int argc, char **argv)
return err;
}
+static int do_load(int argc, char **argv)
+{
+ struct bpf_object *obj;
+ int prog_fd;
+
+ if (argc != 2) {
+ usage();
+ return -1;
+ }
+
+ if (bpf_prog_load(argv[0], BPF_PROG_TYPE_UNSPEC, &obj, &prog_fd)) {
+ p_err("failed to load program\n");
+ return -1;
+ }
+
+ if (do_pin_fd(prog_fd, argv[1])) {
+ p_err("failed to pin program\n");
+ return -1;
+ }
+
+ if (json_output)
+ jsonw_null(json_wtr);
+
+ return 0;
+}
+
static int do_help(int argc, char **argv)
{
if (json_output) {
@@ -647,13 +674,14 @@ static int do_help(int argc, char **argv)
" %s %s dump xlated PROG [{ file FILE | opcodes }]\n"
" %s %s dump jited PROG [{ file FILE | opcodes }]\n"
" %s %s pin PROG FILE\n"
+ " %s %s load SRC FILE\n"
" %s %s help\n"
"\n"
" " HELP_SPEC_PROGRAM "\n"
" " HELP_SPEC_OPTIONS "\n"
"",
bin_name, argv[-2], bin_name, argv[-2], bin_name, argv[-2],
- bin_name, argv[-2], bin_name, argv[-2]);
+ bin_name, argv[-2], bin_name, argv[-2], bin_name, argv[-2]);
return 0;
}
@@ -663,6 +691,7 @@ static const struct cmd cmds[] = {
{ "help", do_help },
{ "dump", do_dump },
{ "pin", do_pin },
+ { "load", do_load },
{ 0 }
};
--
2.14.3
^ permalink raw reply related
* [PATCH v2 net-next 2/4] libbpf: prefer global symbols as bpf program name source
From: Roman Gushchin @ 2017-12-07 18:39 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, kernel-team, ast, daniel, jakub.kicinski, kafai,
guro, Quentin Monnet, David Ahern
In-Reply-To: <20171207183909.16240-1-guro@fb.com>
Libbpf picks the name of the first symbol in the corresponding
elf section to use as a program name. But without taking symbol's
scope into account it may end's up with some local label
as a program name. E.g.:
$ bpftool prog
1: type 15 name LBB0_10 tag 0390a5136ba23f5c
loaded_at Dec 07/17:22 uid 0
xlated 456B not jited memlock 4096B
Fix this by preferring global symbols as program name.
For instance:
$ bpftool prog
1: type 15 name bpf_prog1 tag 0390a5136ba23f5c
loaded_at Dec 07/17:26 uid 0
xlated 456B not jited memlock 4096B
Signed-off-by: Roman Gushchin <guro@fb.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Jakub Kicinski <jakub.kicinski@netronome.com>
Cc: Martin KaFai Lau <kafai@fb.com>
Cc: Quentin Monnet <quentin.monnet@netronome.com>
Cc: David Ahern <dsahern@gmail.com>
---
tools/lib/bpf/libbpf.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 205b7822fa0a..65d0d0aff4fa 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -387,6 +387,8 @@ bpf_object__init_prog_names(struct bpf_object *obj)
continue;
if (sym.st_shndx != prog->idx)
continue;
+ if (GELF_ST_BIND(sym.st_info) != STB_GLOBAL)
+ continue;
name = elf_strptr(obj->efile.elf,
obj->efile.strtabidx,
--
2.14.3
^ permalink raw reply related
* [PATCH v2 net-next 1/4] libbpf: add ability to guess program type based on section name
From: Roman Gushchin @ 2017-12-07 18:39 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, kernel-team, ast, daniel, jakub.kicinski, kafai,
guro, Quentin Monnet, David Ahern
In-Reply-To: <20171207183909.16240-1-guro@fb.com>
The bpf_prog_load() function will guess program type if it's not
specified explicitly. This functionality will be used to implement
loading of different programs without asking a user to specify
the program type. In first order it will be used by bpftool.
Signed-off-by: Roman Gushchin <guro@fb.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Jakub Kicinski <jakub.kicinski@netronome.com>
Cc: Martin KaFai Lau <kafai@fb.com>
Cc: Quentin Monnet <quentin.monnet@netronome.com>
Cc: David Ahern <dsahern@gmail.com>
---
tools/lib/bpf/libbpf.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 51 insertions(+)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 5aa45f89da93..205b7822fa0a 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -1721,6 +1721,45 @@ BPF_PROG_TYPE_FNS(tracepoint, BPF_PROG_TYPE_TRACEPOINT);
BPF_PROG_TYPE_FNS(xdp, BPF_PROG_TYPE_XDP);
BPF_PROG_TYPE_FNS(perf_event, BPF_PROG_TYPE_PERF_EVENT);
+#define BPF_PROG_SEC(string, type) { string, sizeof(string), type }
+static const struct {
+ const char *sec;
+ size_t len;
+ enum bpf_prog_type prog_type;
+} section_names[] = {
+ BPF_PROG_SEC("socket", BPF_PROG_TYPE_SOCKET_FILTER),
+ BPF_PROG_SEC("kprobe/", BPF_PROG_TYPE_KPROBE),
+ BPF_PROG_SEC("kretprobe/", BPF_PROG_TYPE_KPROBE),
+ BPF_PROG_SEC("tracepoint/", BPF_PROG_TYPE_TRACEPOINT),
+ BPF_PROG_SEC("xdp", BPF_PROG_TYPE_XDP),
+ BPF_PROG_SEC("perf_event", BPF_PROG_TYPE_PERF_EVENT),
+ BPF_PROG_SEC("cgroup/skb", BPF_PROG_TYPE_CGROUP_SKB),
+ BPF_PROG_SEC("cgroup/sock", BPF_PROG_TYPE_CGROUP_SOCK),
+ BPF_PROG_SEC("cgroup/dev", BPF_PROG_TYPE_CGROUP_DEVICE),
+ BPF_PROG_SEC("sockops", BPF_PROG_TYPE_SOCK_OPS),
+ BPF_PROG_SEC("sk_skb", BPF_PROG_TYPE_SK_SKB),
+};
+#undef BPF_PROG_SEC
+
+static enum bpf_prog_type bpf_program__guess_type(struct bpf_program *prog)
+{
+ int i;
+
+ if (!prog->section_name)
+ goto err;
+
+ for (i = 0; i < ARRAY_SIZE(section_names); i++)
+ if (strncmp(prog->section_name, section_names[i].sec,
+ section_names[i].len) == 0)
+ return section_names[i].prog_type;
+
+err:
+ pr_warning("failed to guess program type based on section name %s\n",
+ prog->section_name);
+
+ return BPF_PROG_TYPE_UNSPEC;
+}
+
int bpf_map__fd(struct bpf_map *map)
{
return map ? map->fd : -EINVAL;
@@ -1832,6 +1871,18 @@ int bpf_prog_load(const char *file, enum bpf_prog_type type,
return -ENOENT;
}
+ /*
+ * If type is not specified, try to guess it based on
+ * section name.
+ */
+ if (type == BPF_PROG_TYPE_UNSPEC) {
+ type = bpf_program__guess_type(prog);
+ if (type == BPF_PROG_TYPE_UNSPEC) {
+ bpf_object__close(obj);
+ return -EINVAL;
+ }
+ }
+
bpf_program__set_type(prog, type);
err = bpf_object__load(obj);
if (err) {
--
2.14.3
^ permalink raw reply related
* [PATCH v2 net-next 0/4] bpftool: cgroup bpf operations
From: Roman Gushchin @ 2017-12-07 18:39 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, kernel-team, ast, daniel, jakub.kicinski, kafai,
guro, Quentin Monnet, David Ahern
This patchset adds basic cgroup bpf operations to bpftool.
Right now there is no convenient way to perform these operations.
The /samples/bpf/load_sock_ops.c implements attach/detacg operations,
but only for BPF_CGROUP_SOCK_OPS programs. Bps (part of bcc) implements
bpf introspection, but lacks any cgroup-related specific.
I find having a tool to perform these basic operations in the kernel tree
very useful, as it can be used in the corresponding bpf documentation
without creating additional dependencies. And bpftool seems to be
a right tool to extend with such functionality.
v2:
- Added prog load operations
- All cgroup operations are looking like bpftool cgroup <command>
- All cgroup-related stuff is moved to a separate file
- Added support for attach flags
- Added support for attaching/detaching programs by id, pinned name, etc
- Changed cgroup detach arguments order
- Added empty json output for succesful programs
- Style fixed: includes order, strncmp and macroses, error handling
- Added man pages
v1:
https://lwn.net/Articles/740366/
Roman Gushchin (4):
libbpf: add ability to guess program type based on section name
libbpf: prefer global symbols as bpf program name source
bpftool: implement prog load command
bpftool: implement cgroup bpf operations
tools/bpf/bpftool/Documentation/bpftool-cgroup.rst | 92 +++++++
tools/bpf/bpftool/Documentation/bpftool-map.rst | 2 +-
tools/bpf/bpftool/Documentation/bpftool-prog.rst | 12 +-
tools/bpf/bpftool/Documentation/bpftool.rst | 8 +-
tools/bpf/bpftool/cgroup.c | 305 +++++++++++++++++++++
tools/bpf/bpftool/common.c | 71 ++---
tools/bpf/bpftool/main.c | 3 +-
tools/bpf/bpftool/main.h | 2 +
tools/bpf/bpftool/prog.c | 31 ++-
tools/lib/bpf/libbpf.c | 53 ++++
10 files changed, 540 insertions(+), 39 deletions(-)
create mode 100644 tools/bpf/bpftool/Documentation/bpftool-cgroup.rst
create mode 100644 tools/bpf/bpftool/cgroup.c
--
2.14.3
^ permalink raw reply
* Re: [PATCH net-next] bnxt_en: Don't print "Link speed -1 no longer supported" messages.
From: David Miller @ 2017-12-07 18:36 UTC (permalink / raw)
To: michael.chan; +Cc: tbogendoerfer, netdev
In-Reply-To: <1512599482-31273-1-git-send-email-michael.chan@broadcom.com>
From: Michael Chan <michael.chan@broadcom.com>
Date: Wed, 6 Dec 2017 17:31:22 -0500
> On some dual port NICs, the 2 ports have to be configured with compatible
> link speeds. Under some conditions, a port's configured speed may no
> longer be supported. The firmware will send a message to the driver
> when this happens.
>
> Improve this logic that prints out the warning by only printing it if
> we can determine the link speed that is no longer supported. If the
> speed is unknown or it is in autoneg mode, skip the warning message.
>
> Reported-by: Thomas Bogendoerfer <tbogendoerfer@suse.de>
> Signed-off-by: Michael Chan <michael.chan@broadcom.com>
Applied, thanks Michael.
^ permalink raw reply
* Re: [PATCH net-next v2 1/5] net: Introduce NETIF_F_GRO_HW.
From: Alexander Duyck @ 2017-12-07 18:13 UTC (permalink / raw)
To: Michael Chan
Cc: David Miller, Netdev, andrew.gospodarek, Ariel Elior,
everest-linux-l2
In-Reply-To: <1512633815-25037-2-git-send-email-michael.chan@broadcom.com>
On Thu, Dec 7, 2017 at 12:03 AM, Michael Chan <michael.chan@broadcom.com> wrote:
> Introduce NETIF_F_GRO_HW feature flag for NICs that support hardware
> GRO. With this flag, we can now independently turn on or off hardware
> GRO when GRO is on. Previously, drivers were using NETIF_F_GRO to
> control hardware GRO and so it cannot be independently turned on or
> off without affecting GRO.
>
> Hardware GRO (just like GRO) guarantees that packets can be re-segmented
> by TSO/GSO to reconstruct the original packet stream. It is a subset of
> NETIF_F_GRO and depends on it
>
> Since NETIF_F_GRO is not propagated between upper and lower devices,
> NETIF_F_GRO_HW should follow suit since it is a subset of GRO. In other
> words, a lower device can independent have GRO/GRO_HW enabled or disabled
> and no feature propagation is required. This will preserve the current
> GRO behavior.
>
> Cc: Ariel Elior <Ariel.Elior@cavium.com>
> Cc: everest-linux-l2@cavium.com
> Signed-off-by: Michael Chan <michael.chan@broadcom.com>
> ---
> Documentation/networking/netdev-features.txt | 8 ++++++++
> include/linux/netdev_features.h | 3 +++
> net/core/dev.c | 17 +++++++++++++++++
> net/core/ethtool.c | 1 +
> 4 files changed, 29 insertions(+)
>
> diff --git a/Documentation/networking/netdev-features.txt b/Documentation/networking/netdev-features.txt
> index 7413eb0..8f36527 100644
> --- a/Documentation/networking/netdev-features.txt
> +++ b/Documentation/networking/netdev-features.txt
> @@ -163,3 +163,11 @@ This requests that the NIC receive all possible frames, including errored
> frames (such as bad FCS, etc). This can be helpful when sniffing a link with
> bad packets on it. Some NICs may receive more packets if also put into normal
> PROMISC mode.
> +
> +* rx-gro-hw
> +
> +This requests that the NIC enables Hardware GRO (generic receive offload).
> +Hardware GRO is basically the exact reverse of TSO, and is generally
> +stricter than Hardware LRO. A packet stream merged by Hardware GRO must
> +be re-segmentable by GSO or TSO back to the exact original packet stream.
> +Hardware GRO is dependent on GRO and RXCSUM.
> diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h
> index b1b0ca7..db84c51 100644
> --- a/include/linux/netdev_features.h
> +++ b/include/linux/netdev_features.h
> @@ -78,6 +78,8 @@ enum {
> NETIF_F_HW_ESP_TX_CSUM_BIT, /* ESP with TX checksum offload */
> NETIF_F_RX_UDP_TUNNEL_PORT_BIT, /* Offload of RX port for UDP tunnels */
>
> + NETIF_F_GRO_HW_BIT, /* Hardware Generic receive offload */
> +
> /*
> * Add your fresh new feature above and remember to update
> * netdev_features_strings[] in net/core/ethtool.c and maybe
> @@ -97,6 +99,7 @@ enum {
> #define NETIF_F_FRAGLIST __NETIF_F(FRAGLIST)
> #define NETIF_F_FSO __NETIF_F(FSO)
> #define NETIF_F_GRO __NETIF_F(GRO)
> +#define NETIF_F_GRO_HW __NETIF_F(GRO_HW)
> #define NETIF_F_GSO __NETIF_F(GSO)
> #define NETIF_F_GSO_ROBUST __NETIF_F(GSO_ROBUST)
> #define NETIF_F_HIGHDMA __NETIF_F(HIGHDMA)
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 6bea893..7242e5e 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -7405,6 +7405,23 @@ static netdev_features_t netdev_fix_features(struct net_device *dev,
> features &= ~dev->gso_partial_features;
> }
>
> + if (features & NETIF_F_GRO_HW) {
> + /* Hardware GRO depends on GRO and RXCSUM. */
> + if (!(features & NETIF_F_GRO)) {
> + netdev_dbg(dev, "Dropping NETIF_F_GSO_HW since no GRO feature.\n");
> + features &= ~NETIF_F_GRO_HW;
> + }
I'm not sure I agree with this part. The hardware offload should not
be dependent on the software offload. If you are concerned with the
XDP case and such you should probably just look at handling it more
like how TSO is handled with a group of flags that aggregate GRO, LRO,
and GRO_HW into a group of features that must be disabled to support
XDP.
> + if (!(features & NETIF_F_RXCSUM)) {
> + netdev_dbg(dev, "Dropping NETIF_F_GSO_HW since no RXCSUM feature.\n");
> + features &= ~NETIF_F_GRO_HW;
> + }
> + /* Hardware GRO and LRO are mutually exclusive. */
> + if (features & NETIF_F_LRO) {
> + netdev_dbg(dev, "Dropping NETIF_F_LRO since GRO_HW is set.\n");
> + features &= ~NETIF_F_LRO;
> + }
This is going to be problematic. What if you bond one interface that
has LRO and one that has GRO_HW? It seems like this is going to cause
the bond interface to lie and say LRO isn't there when it actually is,
or worse yet it will force features off when they shouldn't be.
> + }
> +
> return features;
> }
>
> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
> index f8fcf45..50a7920 100644
> --- a/net/core/ethtool.c
> +++ b/net/core/ethtool.c
> @@ -73,6 +73,7 @@ int ethtool_op_get_ts_info(struct net_device *dev, struct ethtool_ts_info *info)
> [NETIF_F_LLTX_BIT] = "tx-lockless",
> [NETIF_F_NETNS_LOCAL_BIT] = "netns-local",
> [NETIF_F_GRO_BIT] = "rx-gro",
> + [NETIF_F_GRO_HW_BIT] = "rx-gro-hw",
> [NETIF_F_LRO_BIT] = "rx-lro",
>
> [NETIF_F_TSO_BIT] = "tx-tcp-segmentation",
> --
> 1.8.3.1
>
^ permalink raw reply
* Re: [net-next PATCH 00/14] lockless qdisc series
From: David Miller @ 2017-12-07 18:09 UTC (permalink / raw)
To: john.fastabend
Cc: willemdebruijn.kernel, daniel, eric.dumazet, netdev, jiri,
xiyou.wangcong
In-Reply-To: <20171207173500.5771.41198.stgit@john-Precision-Tower-5810>
From: John Fastabend <john.fastabend@gmail.com>
Date: Thu, 07 Dec 2017 09:53:46 -0800
> This series adds support for building lockless qdiscs.
John, thanks for keeping this going.
^ permalink raw reply
* Re: [PATCH net-next 1/6] net: sched: sch_api: handle generic qdisc errors
From: David Miller @ 2017-12-07 18:08 UTC (permalink / raw)
To: dsahern; +Cc: jhs, aring, xiyou.wangcong, jiri, netdev, kernel
In-Reply-To: <5b8015b8-4fd2-0b1e-2e78-9b318b529f30@gmail.com>
From: David Ahern <dsahern@gmail.com>
Date: Thu, 7 Dec 2017 10:52:01 -0700
> What value are the messages providing above and beyond the standard libc
> strerror(errno)? In the case of ENOMEM, there is nothing in the user can
> do to fix that particular command, so why bloat the code with extraneous
> messages? Similarly with other errors like ENODEV -- if there is only 1
> device in question AND the user specified the device then you do not
> need to augment with an additional error message.
Agreed, all of this stuff is bloat and not useful in any way to the user.
^ permalink raw reply
* Re: [PATCH net 0/3] TCP BBR sampling fixes for loss recovery undo
From: David Miller @ 2017-12-07 18:07 UTC (permalink / raw)
To: ncardwell; +Cc: netdev
In-Reply-To: <CADVnQymVKptA4ZFxfW2M5gHKfZspt_pp2NcWW1ryyaXtrrQeBw@mail.gmail.com>
From: Neal Cardwell <ncardwell@google.com>
Date: Thu, 7 Dec 2017 12:48:40 -0500
> On Thu, Dec 7, 2017 at 12:43 PM, Neal Cardwell <ncardwell@google.com> wrote:
>> This patch series has a few minor bug fixes for cases where spurious
>> loss recoveries can trick BBR estimators into estimating that the
>> available bandwidth is much lower than the true available bandwidth.
>> In both cases the fix here is to just reset the estimator upon loss
>> recovery undo.
>>
>> Neal Cardwell (3):
>> tcp_bbr: record "full bw reached" decision in new full_bw_reached bit
>> tcp_bbr: reset full pipe detection on loss recovery undo
>> tcp_bbr: reset long-term bandwidth sampling on loss recovery undo
>>
>> net/ipv4/tcp_bbr.c | 12 ++++++++++--
>> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> Sorry, I should have mentioned these patches are all stable candidates
> and should have all carried a footer of:
>
> Fixes: 0f8782ea1497 ("tcp_bbr: add BBR congestion control")
>
> David, I can resend a v2 with that footer on each patch if you want.
It is not necessary to resend.
Thanks.
^ permalink raw reply
* [net-next PATCH 14/14] net: sched: pfifo_fast use skb_array
From: John Fastabend @ 2017-12-07 17:58 UTC (permalink / raw)
To: willemdebruijn.kernel, daniel, eric.dumazet, davem
Cc: netdev, jiri, xiyou.wangcong
In-Reply-To: <20171207173500.5771.41198.stgit@john-Precision-Tower-5810>
This converts the pfifo_fast qdisc to use the skb_array data structure
and set the lockless qdisc bit. pfifo_fast is the first qdisc to support
the lockless bit that can be a child of a qdisc requiring locking. So
we add logic to clear the lock bit on initialization in these cases when
the qdisc graft operation occurs.
This also removes the logic used to pick the next band to dequeue from
and instead just checks a per priority array for packets from top priority
to lowest. This might need to be a bit more clever but seems to work
for now.
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
net/sched/sch_api.c | 5 ++
net/sched/sch_generic.c | 140 +++++++++++++++++++++++++++++------------------
2 files changed, 92 insertions(+), 53 deletions(-)
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index 5cb64d2..81f61f5 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -955,6 +955,11 @@ static int qdisc_graft(struct net_device *dev, struct Qdisc *parent,
} else {
const struct Qdisc_class_ops *cops = parent->ops->cl_ops;
+ /* Only support running class lockless if parent is lockless */
+ if (new && (new->flags & TCQ_F_NOLOCK) &&
+ parent && !(parent->flags & TCQ_F_NOLOCK))
+ new->flags &= ~TCQ_F_NOLOCK;
+
err = -EOPNOTSUPP;
if (cops && cops->graft) {
unsigned long cl = cops->find(parent, classid);
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 5ff93c2..ff6a5ac 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -26,6 +26,7 @@
#include <linux/list.h>
#include <linux/slab.h>
#include <linux/if_vlan.h>
+#include <linux/skb_array.h>
#include <net/sch_generic.h>
#include <net/pkt_sched.h>
#include <net/dst.h>
@@ -578,93 +579,93 @@ struct Qdisc_ops noqueue_qdisc_ops __read_mostly = {
/*
* Private data for a pfifo_fast scheduler containing:
- * - queues for the three band
- * - bitmap indicating which of the bands contain skbs
+ * - rings for priority bands
*/
struct pfifo_fast_priv {
- u32 bitmap;
- struct qdisc_skb_head q[PFIFO_FAST_BANDS];
+ struct skb_array q[PFIFO_FAST_BANDS];
};
-/*
- * Convert a bitmap to the first band number where an skb is queued, where:
- * bitmap=0 means there are no skbs on any band.
- * bitmap=1 means there is an skb on band 0.
- * bitmap=7 means there are skbs on all 3 bands, etc.
- */
-static const int bitmap2band[] = {-1, 0, 1, 0, 2, 0, 1, 0};
-
-static inline struct qdisc_skb_head *band2list(struct pfifo_fast_priv *priv,
- int band)
+static inline struct skb_array *band2list(struct pfifo_fast_priv *priv,
+ int band)
{
- return priv->q + band;
+ return &priv->q[band];
}
static int pfifo_fast_enqueue(struct sk_buff *skb, struct Qdisc *qdisc,
struct sk_buff **to_free)
{
- if (qdisc->q.qlen < qdisc_dev(qdisc)->tx_queue_len) {
- int band = prio2band[skb->priority & TC_PRIO_MAX];
- struct pfifo_fast_priv *priv = qdisc_priv(qdisc);
- struct qdisc_skb_head *list = band2list(priv, band);
-
- priv->bitmap |= (1 << band);
- qdisc->q.qlen++;
- return __qdisc_enqueue_tail(skb, qdisc, list);
- }
+ int band = prio2band[skb->priority & TC_PRIO_MAX];
+ struct pfifo_fast_priv *priv = qdisc_priv(qdisc);
+ struct skb_array *q = band2list(priv, band);
+ int err;
- return qdisc_drop(skb, qdisc, to_free);
+ err = skb_array_produce(q, skb);
+
+ if (unlikely(err))
+ return qdisc_drop_cpu(skb, qdisc, to_free);
+
+ qdisc_qstats_cpu_qlen_inc(qdisc);
+ qdisc_qstats_cpu_backlog_inc(qdisc, skb);
+ return NET_XMIT_SUCCESS;
}
static struct sk_buff *pfifo_fast_dequeue(struct Qdisc *qdisc)
{
struct pfifo_fast_priv *priv = qdisc_priv(qdisc);
- int band = bitmap2band[priv->bitmap];
-
- if (likely(band >= 0)) {
- struct qdisc_skb_head *qh = band2list(priv, band);
- struct sk_buff *skb = __qdisc_dequeue_head(qh);
+ struct sk_buff *skb = NULL;
+ int band;
- if (likely(skb != NULL)) {
- qdisc_qstats_backlog_dec(qdisc, skb);
- qdisc_bstats_update(qdisc, skb);
- }
+ for (band = 0; band < PFIFO_FAST_BANDS && !skb; band++) {
+ struct skb_array *q = band2list(priv, band);
- qdisc->q.qlen--;
- if (qh->qlen == 0)
- priv->bitmap &= ~(1 << band);
+ if (__skb_array_empty(q))
+ continue;
- return skb;
+ skb = skb_array_consume_bh(q);
+ }
+ if (likely(skb)) {
+ qdisc_qstats_cpu_backlog_dec(qdisc, skb);
+ qdisc_bstats_cpu_update(qdisc, skb);
+ qdisc_qstats_cpu_qlen_dec(qdisc);
}
- return NULL;
+ return skb;
}
static struct sk_buff *pfifo_fast_peek(struct Qdisc *qdisc)
{
struct pfifo_fast_priv *priv = qdisc_priv(qdisc);
- int band = bitmap2band[priv->bitmap];
+ struct sk_buff *skb = NULL;
+ int band;
- if (band >= 0) {
- struct qdisc_skb_head *qh = band2list(priv, band);
+ for (band = 0; band < PFIFO_FAST_BANDS && !skb; band++) {
+ struct skb_array *q = band2list(priv, band);
- return qh->head;
+ skb = __skb_array_peek(q);
}
- return NULL;
+ return skb;
}
static void pfifo_fast_reset(struct Qdisc *qdisc)
{
- int prio;
+ int i, band;
struct pfifo_fast_priv *priv = qdisc_priv(qdisc);
- for (prio = 0; prio < PFIFO_FAST_BANDS; prio++)
- __qdisc_reset_queue(band2list(priv, prio));
+ for (band = 0; band < PFIFO_FAST_BANDS; band++) {
+ struct skb_array *q = band2list(priv, band);
+ struct sk_buff *skb;
- priv->bitmap = 0;
- qdisc->qstats.backlog = 0;
- qdisc->q.qlen = 0;
+ while ((skb = skb_array_consume_bh(q)) != NULL)
+ kfree_skb(skb);
+ }
+
+ for_each_possible_cpu(i) {
+ struct gnet_stats_queue *q = per_cpu_ptr(qdisc->cpu_qstats, i);
+
+ q->backlog = 0;
+ q->qlen = 0;
+ }
}
static int pfifo_fast_dump(struct Qdisc *qdisc, struct sk_buff *skb)
@@ -682,17 +683,48 @@ static int pfifo_fast_dump(struct Qdisc *qdisc, struct sk_buff *skb)
static int pfifo_fast_init(struct Qdisc *qdisc, struct nlattr *opt)
{
- int prio;
+ unsigned int qlen = qdisc_dev(qdisc)->tx_queue_len;
struct pfifo_fast_priv *priv = qdisc_priv(qdisc);
+ int prio;
+
+ /* guard against zero length rings */
+ if (!qlen)
+ return -EINVAL;
- for (prio = 0; prio < PFIFO_FAST_BANDS; prio++)
- qdisc_skb_head_init(band2list(priv, prio));
+ for (prio = 0; prio < PFIFO_FAST_BANDS; prio++) {
+ struct skb_array *q = band2list(priv, prio);
+ int err;
+
+ err = skb_array_init(q, qlen, GFP_KERNEL);
+ if (err)
+ return -ENOMEM;
+ }
/* Can by-pass the queue discipline */
qdisc->flags |= TCQ_F_CAN_BYPASS;
return 0;
}
+static void pfifo_fast_destroy(struct Qdisc *sch)
+{
+ struct pfifo_fast_priv *priv = qdisc_priv(sch);
+ int prio;
+
+ for (prio = 0; prio < PFIFO_FAST_BANDS; prio++) {
+ struct skb_array *q = band2list(priv, prio);
+
+ /* NULL ring is possible if destroy path is due to a failed
+ * skb_array_init() in pfifo_fast_init() case.
+ */
+ if (!&q->ring.queue)
+ continue;
+ /* Destroy ring but no need to kfree_skb because a call to
+ * pfifo_fast_reset() has already done that work.
+ */
+ ptr_ring_cleanup(&q->ring, NULL);
+ }
+}
+
struct Qdisc_ops pfifo_fast_ops __read_mostly = {
.id = "pfifo_fast",
.priv_size = sizeof(struct pfifo_fast_priv),
@@ -700,9 +732,11 @@ struct Qdisc_ops pfifo_fast_ops __read_mostly = {
.dequeue = pfifo_fast_dequeue,
.peek = pfifo_fast_peek,
.init = pfifo_fast_init,
+ .destroy = pfifo_fast_destroy,
.reset = pfifo_fast_reset,
.dump = pfifo_fast_dump,
.owner = THIS_MODULE,
+ .static_flags = TCQ_F_NOLOCK | TCQ_F_CPUSTATS,
};
EXPORT_SYMBOL(pfifo_fast_ops);
^ permalink raw reply related
* [net-next PATCH 13/14] net: skb_array: expose peek API
From: John Fastabend @ 2017-12-07 17:57 UTC (permalink / raw)
To: willemdebruijn.kernel, daniel, eric.dumazet, davem
Cc: netdev, jiri, xiyou.wangcong
In-Reply-To: <20171207173500.5771.41198.stgit@john-Precision-Tower-5810>
This adds a peek routine to skb_array.h for use with qdisc.
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
include/linux/skb_array.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/include/linux/skb_array.h b/include/linux/skb_array.h
index 8621ffd..c7addf3 100644
--- a/include/linux/skb_array.h
+++ b/include/linux/skb_array.h
@@ -72,6 +72,11 @@ static inline bool __skb_array_empty(struct skb_array *a)
return !__ptr_ring_peek(&a->ring);
}
+static inline struct sk_buff *__skb_array_peek(struct skb_array *a)
+{
+ return __ptr_ring_peek(&a->ring);
+}
+
static inline bool skb_array_empty(struct skb_array *a)
{
return ptr_ring_empty(&a->ring);
^ permalink raw reply related
* [net-next PATCH 12/14] net: sched: add support for TCQ_F_NOLOCK subqueues to sch_mqprio
From: John Fastabend @ 2017-12-07 17:57 UTC (permalink / raw)
To: willemdebruijn.kernel, daniel, eric.dumazet, davem
Cc: netdev, jiri, xiyou.wangcong
In-Reply-To: <20171207173500.5771.41198.stgit@john-Precision-Tower-5810>
The sch_mqprio qdisc creates a sub-qdisc per tx queue which are then
called independently for enqueue and dequeue operations. However
statistics are aggregated and pushed up to the "master" qdisc.
This patch adds support for any of the sub-qdiscs to be per cpu
statistic qdiscs. To handle this case add a check when calculating
stats and aggregate the per cpu stats if needed.
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
net/sched/sch_mq.c | 35 +++++++++++++++---------
net/sched/sch_mqprio.c | 69 +++++++++++++++++++++++++++++++++---------------
2 files changed, 69 insertions(+), 35 deletions(-)
diff --git a/net/sched/sch_mq.c b/net/sched/sch_mq.c
index bc59f05..8cbb5c8 100644
--- a/net/sched/sch_mq.c
+++ b/net/sched/sch_mq.c
@@ -98,33 +98,42 @@ static int mq_dump(struct Qdisc *sch, struct sk_buff *skb)
struct net_device *dev = qdisc_dev(sch);
struct Qdisc *qdisc;
unsigned int ntx;
+ __u32 qlen = 0;
sch->q.qlen = 0;
memset(&sch->bstats, 0, sizeof(sch->bstats));
memset(&sch->qstats, 0, sizeof(sch->qstats));
+ /* MQ supports lockless qdiscs. However, statistics accounting needs
+ * to account for all, none, or a mix of locked and unlocked child
+ * qdiscs. Percpu stats are added to counters in-band and locking
+ * qdisc totals are added at end.
+ */
for (ntx = 0; ntx < dev->num_tx_queues; ntx++) {
- struct gnet_stats_basic_cpu __percpu *cpu_bstats = NULL;
- struct gnet_stats_queue __percpu *cpu_qstats = NULL;
- __u32 qlen = 0;
-
qdisc = netdev_get_tx_queue(dev, ntx)->qdisc_sleeping;
spin_lock_bh(qdisc_lock(qdisc));
if (qdisc_is_percpu_stats(qdisc)) {
- cpu_bstats = qdisc->cpu_bstats;
- cpu_qstats = qdisc->cpu_qstats;
+ qlen = qdisc_qlen_sum(qdisc);
+ __gnet_stats_copy_basic(NULL, &sch->bstats,
+ qdisc->cpu_bstats,
+ &qdisc->bstats);
+ __gnet_stats_copy_queue(&sch->qstats,
+ qdisc->cpu_qstats,
+ &qdisc->qstats, qlen);
+ } else {
+ sch->q.qlen += qdisc->q.qlen;
+ sch->bstats.bytes += qdisc->bstats.bytes;
+ sch->bstats.packets += qdisc->bstats.packets;
+ sch->qstats.backlog += qdisc->qstats.backlog;
+ sch->qstats.drops += qdisc->qstats.drops;
+ sch->qstats.requeues += qdisc->qstats.requeues;
+ sch->qstats.overlimits += qdisc->qstats.overlimits;
}
- qlen = qdisc_qlen_sum(qdisc);
-
- __gnet_stats_copy_basic(NULL, &sch->bstats,
- cpu_bstats, &qdisc->bstats);
- __gnet_stats_copy_queue(&sch->qstats,
- cpu_qstats, &qdisc->qstats, qlen);
-
spin_unlock_bh(qdisc_lock(qdisc));
}
+
return 0;
}
diff --git a/net/sched/sch_mqprio.c b/net/sched/sch_mqprio.c
index b85885a9..8622745 100644
--- a/net/sched/sch_mqprio.c
+++ b/net/sched/sch_mqprio.c
@@ -388,22 +388,40 @@ static int mqprio_dump(struct Qdisc *sch, struct sk_buff *skb)
struct nlattr *nla = (struct nlattr *)skb_tail_pointer(skb);
struct tc_mqprio_qopt opt = { 0 };
struct Qdisc *qdisc;
- unsigned int i;
+ unsigned int ntx, tc;
sch->q.qlen = 0;
memset(&sch->bstats, 0, sizeof(sch->bstats));
memset(&sch->qstats, 0, sizeof(sch->qstats));
- for (i = 0; i < dev->num_tx_queues; i++) {
- qdisc = rtnl_dereference(netdev_get_tx_queue(dev, i)->qdisc);
+ /* MQ supports lockless qdiscs. However, statistics accounting needs
+ * to account for all, none, or a mix of locked and unlocked child
+ * qdiscs. Percpu stats are added to counters in-band and locking
+ * qdisc totals are added at end.
+ */
+ for (ntx = 0; ntx < dev->num_tx_queues; ntx++) {
+ qdisc = netdev_get_tx_queue(dev, ntx)->qdisc_sleeping;
spin_lock_bh(qdisc_lock(qdisc));
- sch->q.qlen += qdisc->q.qlen;
- sch->bstats.bytes += qdisc->bstats.bytes;
- sch->bstats.packets += qdisc->bstats.packets;
- sch->qstats.backlog += qdisc->qstats.backlog;
- sch->qstats.drops += qdisc->qstats.drops;
- sch->qstats.requeues += qdisc->qstats.requeues;
- sch->qstats.overlimits += qdisc->qstats.overlimits;
+
+ if (qdisc_is_percpu_stats(qdisc)) {
+ __u32 qlen = qdisc_qlen_sum(qdisc);
+
+ __gnet_stats_copy_basic(NULL, &sch->bstats,
+ qdisc->cpu_bstats,
+ &qdisc->bstats);
+ __gnet_stats_copy_queue(&sch->qstats,
+ qdisc->cpu_qstats,
+ &qdisc->qstats, qlen);
+ } else {
+ sch->q.qlen += qdisc->q.qlen;
+ sch->bstats.bytes += qdisc->bstats.bytes;
+ sch->bstats.packets += qdisc->bstats.packets;
+ sch->qstats.backlog += qdisc->qstats.backlog;
+ sch->qstats.drops += qdisc->qstats.drops;
+ sch->qstats.requeues += qdisc->qstats.requeues;
+ sch->qstats.overlimits += qdisc->qstats.overlimits;
+ }
+
spin_unlock_bh(qdisc_lock(qdisc));
}
@@ -411,9 +429,9 @@ static int mqprio_dump(struct Qdisc *sch, struct sk_buff *skb)
memcpy(opt.prio_tc_map, dev->prio_tc_map, sizeof(opt.prio_tc_map));
opt.hw = priv->hw_offload;
- for (i = 0; i < netdev_get_num_tc(dev); i++) {
- opt.count[i] = dev->tc_to_txq[i].count;
- opt.offset[i] = dev->tc_to_txq[i].offset;
+ for (tc = 0; tc < netdev_get_num_tc(dev); tc++) {
+ opt.count[tc] = dev->tc_to_txq[tc].count;
+ opt.offset[tc] = dev->tc_to_txq[tc].offset;
}
if (nla_put(skb, TCA_OPTIONS, NLA_ALIGN(sizeof(opt)), &opt))
@@ -495,7 +513,6 @@ static int mqprio_dump_class_stats(struct Qdisc *sch, unsigned long cl,
if (cl >= TC_H_MIN_PRIORITY) {
int i;
__u32 qlen = 0;
- struct Qdisc *qdisc;
struct gnet_stats_queue qstats = {0};
struct gnet_stats_basic_packed bstats = {0};
struct net_device *dev = qdisc_dev(sch);
@@ -511,18 +528,26 @@ static int mqprio_dump_class_stats(struct Qdisc *sch, unsigned long cl,
for (i = tc.offset; i < tc.offset + tc.count; i++) {
struct netdev_queue *q = netdev_get_tx_queue(dev, i);
+ struct Qdisc *qdisc = rtnl_dereference(q->qdisc);
+ struct gnet_stats_basic_cpu __percpu *cpu_bstats = NULL;
+ struct gnet_stats_queue __percpu *cpu_qstats = NULL;
- qdisc = rtnl_dereference(q->qdisc);
spin_lock_bh(qdisc_lock(qdisc));
- qlen += qdisc->q.qlen;
- bstats.bytes += qdisc->bstats.bytes;
- bstats.packets += qdisc->bstats.packets;
- qstats.backlog += qdisc->qstats.backlog;
- qstats.drops += qdisc->qstats.drops;
- qstats.requeues += qdisc->qstats.requeues;
- qstats.overlimits += qdisc->qstats.overlimits;
+ if (qdisc_is_percpu_stats(qdisc)) {
+ cpu_bstats = qdisc->cpu_bstats;
+ cpu_qstats = qdisc->cpu_qstats;
+ }
+
+ qlen = qdisc_qlen_sum(qdisc);
+ __gnet_stats_copy_basic(NULL, &sch->bstats,
+ cpu_bstats, &qdisc->bstats);
+ __gnet_stats_copy_queue(&sch->qstats,
+ cpu_qstats,
+ &qdisc->qstats,
+ qlen);
spin_unlock_bh(qdisc_lock(qdisc));
}
+
/* Reclaim root sleeping lock before completing stats */
if (d->lock)
spin_lock_bh(d->lock);
^ permalink raw reply related
* [net-next PATCH 11/14] net: sched: add support for TCQ_F_NOLOCK subqueues to sch_mq
From: John Fastabend @ 2017-12-07 17:57 UTC (permalink / raw)
To: willemdebruijn.kernel, daniel, eric.dumazet, davem
Cc: netdev, jiri, xiyou.wangcong
In-Reply-To: <20171207173500.5771.41198.stgit@john-Precision-Tower-5810>
The sch_mq qdisc creates a sub-qdisc per tx queue which are then
called independently for enqueue and dequeue operations. However
statistics are aggregated and pushed up to the "master" qdisc.
This patch adds support for any of the sub-qdiscs to be per cpu
statistic qdiscs. To handle this case add a check when calculating
stats and aggregate the per cpu stats if needed.
Also exports __gnet_stats_copy_queue() to use as a helper function.
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
include/net/gen_stats.h | 3 +++
net/core/gen_stats.c | 9 +++++----
net/sched/sch_mq.c | 25 ++++++++++++++++++-------
3 files changed, 26 insertions(+), 11 deletions(-)
diff --git a/include/net/gen_stats.h b/include/net/gen_stats.h
index 304f7aa..0304ba2 100644
--- a/include/net/gen_stats.h
+++ b/include/net/gen_stats.h
@@ -49,6 +49,9 @@ int gnet_stats_copy_rate_est(struct gnet_dump *d,
int gnet_stats_copy_queue(struct gnet_dump *d,
struct gnet_stats_queue __percpu *cpu_q,
struct gnet_stats_queue *q, __u32 qlen);
+void __gnet_stats_copy_queue(struct gnet_stats_queue *qstats,
+ const struct gnet_stats_queue __percpu *cpu_q,
+ const struct gnet_stats_queue *q, __u32 qlen);
int gnet_stats_copy_app(struct gnet_dump *d, void *st, int len);
int gnet_stats_finish_copy(struct gnet_dump *d);
diff --git a/net/core/gen_stats.c b/net/core/gen_stats.c
index 87f2855..b2b2323b 100644
--- a/net/core/gen_stats.c
+++ b/net/core/gen_stats.c
@@ -252,10 +252,10 @@
}
}
-static void __gnet_stats_copy_queue(struct gnet_stats_queue *qstats,
- const struct gnet_stats_queue __percpu *cpu,
- const struct gnet_stats_queue *q,
- __u32 qlen)
+void __gnet_stats_copy_queue(struct gnet_stats_queue *qstats,
+ const struct gnet_stats_queue __percpu *cpu,
+ const struct gnet_stats_queue *q,
+ __u32 qlen)
{
if (cpu) {
__gnet_stats_copy_queue_cpu(qstats, cpu);
@@ -269,6 +269,7 @@ static void __gnet_stats_copy_queue(struct gnet_stats_queue *qstats,
qstats->qlen = qlen;
}
+EXPORT_SYMBOL(__gnet_stats_copy_queue);
/**
* gnet_stats_copy_queue - copy queue statistics into statistics TLV
diff --git a/net/sched/sch_mq.c b/net/sched/sch_mq.c
index 213b586..bc59f05 100644
--- a/net/sched/sch_mq.c
+++ b/net/sched/sch_mq.c
@@ -17,6 +17,7 @@
#include <linux/skbuff.h>
#include <net/netlink.h>
#include <net/pkt_sched.h>
+#include <net/sch_generic.h>
struct mq_sched {
struct Qdisc **qdiscs;
@@ -103,15 +104,25 @@ static int mq_dump(struct Qdisc *sch, struct sk_buff *skb)
memset(&sch->qstats, 0, sizeof(sch->qstats));
for (ntx = 0; ntx < dev->num_tx_queues; ntx++) {
+ struct gnet_stats_basic_cpu __percpu *cpu_bstats = NULL;
+ struct gnet_stats_queue __percpu *cpu_qstats = NULL;
+ __u32 qlen = 0;
+
qdisc = netdev_get_tx_queue(dev, ntx)->qdisc_sleeping;
spin_lock_bh(qdisc_lock(qdisc));
- sch->q.qlen += qdisc->q.qlen;
- sch->bstats.bytes += qdisc->bstats.bytes;
- sch->bstats.packets += qdisc->bstats.packets;
- sch->qstats.backlog += qdisc->qstats.backlog;
- sch->qstats.drops += qdisc->qstats.drops;
- sch->qstats.requeues += qdisc->qstats.requeues;
- sch->qstats.overlimits += qdisc->qstats.overlimits;
+
+ if (qdisc_is_percpu_stats(qdisc)) {
+ cpu_bstats = qdisc->cpu_bstats;
+ cpu_qstats = qdisc->cpu_qstats;
+ }
+
+ qlen = qdisc_qlen_sum(qdisc);
+
+ __gnet_stats_copy_basic(NULL, &sch->bstats,
+ cpu_bstats, &qdisc->bstats);
+ __gnet_stats_copy_queue(&sch->qstats,
+ cpu_qstats, &qdisc->qstats, qlen);
+
spin_unlock_bh(qdisc_lock(qdisc));
}
return 0;
^ permalink raw reply related
* [net-next PATCH 10/14] net: sched: helpers to sum qlen and qlen for per cpu logic
From: John Fastabend @ 2017-12-07 17:57 UTC (permalink / raw)
To: willemdebruijn.kernel, daniel, eric.dumazet, davem
Cc: netdev, jiri, xiyou.wangcong
In-Reply-To: <20171207173500.5771.41198.stgit@john-Precision-Tower-5810>
Add qdisc qlen helper routines for lockless qdiscs to use.
The qdisc qlen is no longer used in the hotpath but it is reported
via stats query on the qdisc so it still needs to be tracked. This
adds the per cpu operations needed along with a helper to return
the summation of per cpu stats.
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
include/net/sch_generic.h | 20 ++++++++++++++++++++
net/sched/sch_api.c | 3 ++-
2 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 4717c4b..2fbae2c9 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -291,11 +291,31 @@ static inline void qdisc_cb_private_validate(const struct sk_buff *skb, int sz)
BUILD_BUG_ON(sizeof(qcb->data) < sz);
}
+static inline int qdisc_qlen_cpu(const struct Qdisc *q)
+{
+ return this_cpu_ptr(q->cpu_qstats)->qlen;
+}
+
static inline int qdisc_qlen(const struct Qdisc *q)
{
return q->q.qlen;
}
+static inline int qdisc_qlen_sum(const struct Qdisc *q)
+{
+ __u32 qlen = 0;
+ int i;
+
+ if (q->flags & TCQ_F_NOLOCK) {
+ for_each_possible_cpu(i)
+ qlen += per_cpu_ptr(q->cpu_qstats, i)->qlen;
+ } else {
+ qlen = q->q.qlen;
+ }
+
+ return qlen;
+}
+
static inline struct qdisc_skb_cb *qdisc_skb_cb(const struct sk_buff *skb)
{
return (struct qdisc_skb_cb *)skb->cb;
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index b6c4f53..5cb64d2 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -797,7 +797,8 @@ static int tc_fill_qdisc(struct sk_buff *skb, struct Qdisc *q, u32 clid,
goto nla_put_failure;
if (q->ops->dump && q->ops->dump(q, skb) < 0)
goto nla_put_failure;
- qlen = q->q.qlen;
+
+ qlen = qdisc_qlen_sum(q);
stab = rtnl_dereference(q->stab);
if (stab && qdisc_dump_stab(skb, stab) < 0)
^ permalink raw reply related
* [net-next PATCH 09/14] net: sched: check for frozen queue before skb_bad_txq check
From: John Fastabend @ 2017-12-07 17:56 UTC (permalink / raw)
To: willemdebruijn.kernel, daniel, eric.dumazet, davem
Cc: netdev, jiri, xiyou.wangcong
In-Reply-To: <20171207173500.5771.41198.stgit@john-Precision-Tower-5810>
I can not think of any reason to pull the bad txq skb off the qdisc if
the txq we plan to send this on is still frozen. So check for frozen
queue first and abort before dequeuing either skb_bad_txq skb or
normal qdisc dequeue() skb.
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
net/sched/sch_generic.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 84cef05..5ff93c2 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -204,7 +204,7 @@ static struct sk_buff *dequeue_skb(struct Qdisc *q, bool *validate,
int *packets)
{
const struct netdev_queue *txq = q->dev_queue;
- struct sk_buff *skb;
+ struct sk_buff *skb = NULL;
*packets = 1;
if (unlikely(!skb_queue_empty(&q->gso_skb))) {
@@ -248,12 +248,15 @@ static struct sk_buff *dequeue_skb(struct Qdisc *q, bool *validate,
}
validate:
*validate = true;
+
+ if ((q->flags & TCQ_F_ONETXQUEUE) &&
+ netif_xmit_frozen_or_stopped(txq))
+ return skb;
+
skb = qdisc_dequeue_skb_bad_txq(q);
if (unlikely(skb))
goto bulk;
- if (!(q->flags & TCQ_F_ONETXQUEUE) ||
- !netif_xmit_frozen_or_stopped(txq))
- skb = q->dequeue(q);
+ skb = q->dequeue(q);
if (skb) {
bulk:
if (qdisc_may_bulk(q))
^ 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