* Re: [PATCH net-next v2 05/14] net: mvpp2: do not force the link mode
From: Antoine Tenart @ 2017-08-28 9:40 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Marcin Wojtas, Antoine Tenart, David S. Miller, kishon,
Andrew Lunn, Jason Cooper, Sebastian Hesselbarth,
Gregory Clément, Thomas Petazzoni, nadavh, linux-kernel,
Stefan Chulski, Miquèl Raynal, netdev
In-Reply-To: <20170828085152.GF20805@n2100.armlinux.org.uk>
[-- Attachment #1: Type: text/plain, Size: 1356 bytes --]
On Mon, Aug 28, 2017 at 09:51:52AM +0100, Russell King - ARM Linux wrote:
> On Mon, Aug 28, 2017 at 10:38:37AM +0200, Marcin Wojtas wrote:
> >
> > Can you be 100% sure that when using SGMII with PHY's (like Marvell
> > Alaska 88E1xxx series), is in-band link information always available?
> > I'd be very cautious with such assumption and use in-band management
> > only when set in the DT, like mvneta. I think phylib can properly can
> > do its work when MDIO connection is provided on the board.
>
> There is another issue to be aware of: if you're wanting to use flow
> control autonegotiation, that is not carried across SGMII's in-band
> signalling. If you want to use SGMII's in-band signalling for the
> duplex and speed information, you still need phylib's notification
> to properly set the flow control.
>
> <plug>
> Switching mvpp2 to use phylink (which is needed for the 1G SFP slot on
> mcbin) will handle all this for you - dealing with both in-band and
> out-of-band negotiation methods, and combining them in the appropriate
> manner for the selected operation mode.
> </plug>
So probably the best move here is to remove this patch, and wait for the
phylink support in the PPv2 driver.
Thanks!
Antoine
--
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH net-next 1/4] net: Add SRIOV VGT+ support
From: Saeed Mahameed @ 2017-08-28 9:50 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Saeed Mahameed, David S. Miller, Linux Netdev List,
Eugenia Emantayev, Mohamad Haj Yahia
In-Reply-To: <20170827203857.369c2b16@cakuba>
On Mon, Aug 28, 2017 at 3:38 AM, Jakub Kicinski <kubakici@wp.pl> wrote:
> On Sun, 27 Aug 2017 14:06:15 +0300, Saeed Mahameed wrote:
>> From: Mohamad Haj Yahia <mohamad@mellanox.com>
>>
>> VGT+ is a security feature that gives the administrator the ability of
>> controlling the allowed vlan-ids list that can be transmitted/received
>> from/to the VF.
>> The allowed vlan-ids list is called "trunk".
>> Admin can add/remove a range of allowed vlan-ids via iptool.
>> Example:
>> After this series of configuration :
>> 1) ip link set eth3 vf 0 trunk add 10 100 (allow vlan-id 10-100, default tpid 0x8100)
>> 2) ip link set eth3 vf 0 trunk add 105 proto 802.1q (allow vlan-id 105 tpid 0x8100)
>> 3) ip link set eth3 vf 0 trunk add 105 proto 802.1ad (allow vlan-id 105 tpid 0x88a8)
>> 4) ip link set eth3 vf 0 trunk rem 90 (block vlan-id 90)
>> 5) ip link set eth3 vf 0 trunk rem 50 60 (block vlan-ids 50-60)
>>
>> The VF 0 can only communicate on vlan-ids: 10-49,61-89,91-100,105 with
>> tpid 0x8100 and vlan-id 105 with tpid 0x88a8.
>>
>> For this purpose we added the following netlink sr-iov commands:
>>
>> 1) IFLA_VF_VLAN_RANGE: used to add/remove allowed vlan-ids range.
>> We added the ifla_vf_vlan_range struct to specify the range we want to
>> add/remove from the userspace.
>> We added ndo_add_vf_vlan_trunk_range and ndo_del_vf_vlan_trunk_range
>> netdev ops to add/remove allowed vlan-ids range in the netdev.
>>
>> 2) IFLA_VF_VLAN_TRUNK: used to query the allowed vlan-ids trunk.
>> We added trunk bitmap to the ifla_vf_info struct to get the current
>> allowed vlan-ids trunk from the netdev.
>> We added ifla_vf_vlan_trunk struct for sending the allowed vlan-ids
>> trunk to the userspace.
>>
>> Signed-off-by: Mohamad Haj Yahia <mohamad@mellanox.com>
>> Signed-off-by: Eugenia Emantayev <eugenia@mellanox.com>
>> Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
>
> Interesting work, I have some minor questions if you don't mind :)
>
Hi Jakub, Thanks for the review.
> I was under impression that "trunk" is a vendor-specific term, would it
> make sense to drop it from the APIs?
>
Well, the term trunk is widely used in switches APIs and since those
patches refer to SRIOV architecture
where an E-Switch is running in the HW level operating as an L2
switch, I think it makes sense to use the same term for the
same functionality we already have in the switches.
>> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
>> index 8d062c58d5cb..3aa895c5fbc1 100644
>> --- a/include/uapi/linux/if_link.h
>> +++ b/include/uapi/linux/if_link.h
>> @@ -168,6 +168,8 @@ enum {
>> #ifndef __KERNEL__
>> #define IFLA_RTA(r) ((struct rtattr*)(((char*)(r)) + NLMSG_ALIGN(sizeof(struct ifinfomsg))))
>> #define IFLA_PAYLOAD(n) NLMSG_PAYLOAD(n,sizeof(struct ifinfomsg))
>> +#define BITS_PER_BYTE 8
>> +#define DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))
>> #endif
>>
>> enum {
>> @@ -645,6 +647,8 @@ enum {
>> IFLA_VF_IB_NODE_GUID, /* VF Infiniband node GUID */
>> IFLA_VF_IB_PORT_GUID, /* VF Infiniband port GUID */
>> IFLA_VF_VLAN_LIST, /* nested list of vlans, option for QinQ */
>> + IFLA_VF_VLAN_RANGE, /* add/delete vlan range filtering */
>> + IFLA_VF_VLAN_TRUNK, /* vlan trunk filtering */
>> __IFLA_VF_MAX,
>> };
>>
>> @@ -669,6 +673,7 @@ enum {
>>
>> #define IFLA_VF_VLAN_INFO_MAX (__IFLA_VF_VLAN_INFO_MAX - 1)
>> #define MAX_VLAN_LIST_LEN 1
>> +#define VF_VLAN_N_VID 4096
>>
>> struct ifla_vf_vlan_info {
>> __u32 vf;
>> @@ -677,6 +682,21 @@ struct ifla_vf_vlan_info {
>> __be16 vlan_proto; /* VLAN protocol either 802.1Q or 802.1ad */
>> };
>>
>> +struct ifla_vf_vlan_range {
>> + __u32 vf;
>> + __u32 start_vid; /* 1 - 4095 */
>> + __u32 end_vid; /* 1 - 4095 */
>> + __u32 setting;
>> + __be16 vlan_proto; /* VLAN protocol either 802.1Q or 802.1ad */
>> +};
>> +
>> +#define VF_VLAN_BITMAP DIV_ROUND_UP(VF_VLAN_N_VID, sizeof(__u64) * BITS_PER_BYTE)
>> +struct ifla_vf_vlan_trunk {
>> + __u32 vf;
>> + __u64 allowed_vlans_8021q_bm[VF_VLAN_BITMAP];
>> + __u64 allowed_vlans_8021ad_bm[VF_VLAN_BITMAP];
>> +};
>
> Would you mind explaining why you chose to make the API asymmetrical
> like that? I mean the set operation is range-based, yet the get
> returns a bitmask. You seem to solely depend on the bitmasks in the
> driver anyway...
>
It is not about driver dependency, simply we would like to store the
allowed vlan in a simple data structure and bitmap is
the simplest form, otherwise we would complicate the APIs to transfer
Lists and ranges back and forth between userspace and kernel.
>> struct ifla_vf_tx_rate {
>> __u32 vf;
>> __u32 rate; /* Max TX bandwidth in Mbps, 0 disables throttling */
>> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
>> index a78fd61da0ec..56909f11d88e 100644
>> --- a/net/core/rtnetlink.c
>> +++ b/net/core/rtnetlink.c
>> @@ -827,6 +827,7 @@ static inline int rtnl_vfinfo_size(const struct net_device *dev,
>> nla_total_size(MAX_VLAN_LIST_LEN *
>> sizeof(struct ifla_vf_vlan_info)) +
>> nla_total_size(sizeof(struct ifla_vf_spoofchk)) +
>> + nla_total_size(sizeof(struct ifla_vf_vlan_trunk)) +
>> nla_total_size(sizeof(struct ifla_vf_tx_rate)) +
>> nla_total_size(sizeof(struct ifla_vf_rate)) +
>> nla_total_size(sizeof(struct ifla_vf_link_state)) +
>> @@ -1098,31 +1099,43 @@ static noinline_for_stack int rtnl_fill_vfinfo(struct sk_buff *skb,
>> struct ifla_vf_link_state vf_linkstate;
>> struct ifla_vf_vlan_info vf_vlan_info;
>> struct ifla_vf_spoofchk vf_spoofchk;
>> + struct ifla_vf_vlan_trunk *vf_trunk;
>> struct ifla_vf_tx_rate vf_tx_rate;
>> struct ifla_vf_stats vf_stats;
>> struct ifla_vf_trust vf_trust;
>> struct ifla_vf_vlan vf_vlan;
>> struct ifla_vf_rate vf_rate;
>> struct ifla_vf_mac vf_mac;
>> - struct ifla_vf_info ivi;
>> + struct ifla_vf_info *ivi;
>>
>> - memset(&ivi, 0, sizeof(ivi));
>> + ivi = kzalloc(sizeof(*ivi), GFP_KERNEL);
>> + if (!ivi)
>> + return -ENOMEM;
>
> In the future please try to split code adjustments like allocating ivi
> here into a separate patch. Makes the changes a little more obvious to
> read.
Since we extended ivi struct, it passed the stack limit, so we did it
in the same patch,
but I agree it would simplify the review to break it into two pieces.
^ permalink raw reply
* Re: [PATCH net-next 3/4] net/core: Add violation counters to VF statisctics
From: Saeed Mahameed @ 2017-08-28 9:52 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Saeed Mahameed, David S. Miller, Linux Netdev List,
Eugenia Emantayev
In-Reply-To: <20170827204306.0fb98e31@cakuba>
On Mon, Aug 28, 2017 at 3:43 AM, Jakub Kicinski <kubakici@wp.pl> wrote:
> On Sun, 27 Aug 2017 14:06:17 +0300, Saeed Mahameed wrote:
>> From: Eugenia Emantayev <eugenia@mellanox.com>
>>
>> Add receive and transmit violation counters to be
>> displayed in iproute2 VF statistics.
>>
>> Signed-off-by: Eugenia Emantayev <eugenia@mellanox.com>
>> Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
>> ---
>> include/linux/if_link.h | 2 ++
>> include/uapi/linux/if_link.h | 2 ++
>> net/core/rtnetlink.c | 10 +++++++++-
>> 3 files changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/linux/if_link.h b/include/linux/if_link.h
>> index da70af27e42e..ebf3448acb5b 100644
>> --- a/include/linux/if_link.h
>> +++ b/include/linux/if_link.h
>> @@ -12,6 +12,8 @@ struct ifla_vf_stats {
>> __u64 tx_bytes;
>> __u64 broadcast;
>> __u64 multicast;
>> + __u64 rx_dropped;
>> + __u64 tx_dropped;
>
> I'm a little concerned that you call those violation counters in the
> commit message. Do you expect them to only be used if the VF traffic
> indeed violates some admin-set rules? I would imaging HW/FW may drop
> frames in certain situations and naming the counters *_dropped suggests
> it would be OK to increment them even if the drop reason was not any
> sort of violation. Would you mind clarifying?
Yes, the rx/tx_dropped counters serve as a general purpose VF drop
counter including VF violations.
We will fix the commit message.
Thanks !
Saeed.
^ permalink raw reply
* Re: DSA support for Micrel KSZ8895
From: Pavel Machek @ 2017-08-28 10:14 UTC (permalink / raw)
To: Woojung.Huh
Cc: nathan.leigh.conrad, vivien.didelot, f.fainelli, netdev,
linux-kernel, Tristram.Ha, andrew
In-Reply-To: <9235D6609DB808459E95D78E17F2E43D40B05FFF@CHN-SV-EXMX02.mchp-main.com>
[-- Attachment #1: Type: text/plain, Size: 832 bytes --]
Hi!
> > > I'll forward your email to our support.
> > > AFAIK, KSZ8895 has different register mapping from KSZ9477,
> > > it will be more than ID changes in current driver.
> >
> > More than ID changes, indeed. As layout is completely different, it
> > looks like different source file will be needed for support.
> >
> > I'm not nearly there; but I can ifconfig lanX up, already, and perform
> > some pings.
> >
> > Any ideas how to do the work in a way to minimize code duplication are
> > welcome...
>
> Which version do you use to create patch?
> Getting error when applying patch to the latest net-next.
I'm working at 4.13-rc.
Best regards,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [PATCH] dt-binding: net/phy: fix interrupts description
From: Baruch Siach @ 2017-08-28 10:20 UTC (permalink / raw)
To: David S. Miller
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
Rob Herring, Mark Rutland
In-Reply-To: <b3756346473feadfeba70ecb71960cad48e66621.1503468660.git.baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org>
Hi Dave,
On Wed, Aug 23, 2017 at 09:11:00AM +0300, Baruch Siach wrote:
> Commit b053dc5a722ea (powerpc: Refactor device tree binding) split the
> Ethernet PHY binding documentation out of the big booting-without-of.txt
> file, leaving a dangling reference to "section 2" in the 'interrupts'
> property description. Drop that reference, and make the description look
> more like the rest.
>
> While at it, make the example interrupt-parent phandle look more like a
> real world phandle, and use an IRQ_TYPE_ macro for the 'interrupts'
> type.
This patch is now marked 'Not Applicable' in the netdev patchwork. Why is
that? Should it go through some other tree?
baruch
> Signed-off-by: Baruch Siach <baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org>
> ---
> Documentation/devicetree/bindings/net/phy.txt | 10 +++-------
> 1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/phy.txt b/Documentation/devicetree/bindings/net/phy.txt
> index b55857696fc3..86ba72af6163 100644
> --- a/Documentation/devicetree/bindings/net/phy.txt
> +++ b/Documentation/devicetree/bindings/net/phy.txt
> @@ -2,11 +2,7 @@ PHY nodes
>
> Required properties:
>
> - - interrupts : <a b> where a is the interrupt number and b is a
> - field that represents an encoding of the sense and level
> - information for the interrupt. This should be encoded based on
> - the information in section 2) depending on the type of interrupt
> - controller you have.
> + - interrupts : interrupt specifier for the sole interrupt.
> - interrupt-parent : the phandle for the interrupt controller that
> services interrupts for this device.
> - reg : The ID number for the phy, usually a small integer
> @@ -56,7 +52,7 @@ Example:
>
> ethernet-phy@0 {
> compatible = "ethernet-phy-id0141.0e90", "ethernet-phy-ieee802.3-c22";
> - interrupt-parent = <40000>;
> - interrupts = <35 1>;
> + interrupt-parent = <&PIC>;
> + interrupts = <35 IRQ_TYPE_EDGE_RISING>;
> reg = <0>;
> };
--
http://baruch.siach.name/blog/ ~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org - tel: +972.52.368.4656, http://www.tkos.co.il -
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH net-next v2 05/14] net: mvpp2: do not force the link mode
From: Russell King - ARM Linux @ 2017-08-28 11:06 UTC (permalink / raw)
To: Antoine Tenart
Cc: Marcin Wojtas, David S. Miller, kishon, Andrew Lunn, Jason Cooper,
Sebastian Hesselbarth, Gregory Clément, Thomas Petazzoni,
nadavh, linux-kernel, Stefan Chulski, Miquèl Raynal, netdev
In-Reply-To: <20170828094051.GE2568@kwain>
On Mon, Aug 28, 2017 at 11:40:51AM +0200, Antoine Tenart wrote:
> On Mon, Aug 28, 2017 at 09:51:52AM +0100, Russell King - ARM Linux wrote:
> > On Mon, Aug 28, 2017 at 10:38:37AM +0200, Marcin Wojtas wrote:
> > >
> > > Can you be 100% sure that when using SGMII with PHY's (like Marvell
> > > Alaska 88E1xxx series), is in-band link information always available?
> > > I'd be very cautious with such assumption and use in-band management
> > > only when set in the DT, like mvneta. I think phylib can properly can
> > > do its work when MDIO connection is provided on the board.
> >
> > There is another issue to be aware of: if you're wanting to use flow
> > control autonegotiation, that is not carried across SGMII's in-band
> > signalling. If you want to use SGMII's in-band signalling for the
> > duplex and speed information, you still need phylib's notification
> > to properly set the flow control.
> >
> > <plug>
> > Switching mvpp2 to use phylink (which is needed for the 1G SFP slot on
> > mcbin) will handle all this for you - dealing with both in-band and
> > out-of-band negotiation methods, and combining them in the appropriate
> > manner for the selected operation mode.
> > </plug>
>
> So probably the best move here is to remove this patch, and wait for the
> phylink support in the PPv2 driver.
I've nothing on that specifically for the mvpp2 driver - what I have is
for mvneta and the Marvell mvpp2x driver, with GMAC support extracted
from mvneta (that last bit is rather dirty at the moment so not
published anywhere, and doesn't cater for PP v2.1 at all.)
I ought to have posted the mvneta part of the phylink patches, but I
didn't get around to it early enough in this cycle - there are probably
quite a number of conflicts with net-next now, so I think it's too late
to submit it for mainline.
I know Andrew has already looked at them in my git tree as part of the
review of phylink when that was merged - which should be adequate to
give an example of how to implement it for the mainline PP v2 driver.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
^ permalink raw reply
* Re: [PATCH net-next v2 05/14] net: mvpp2: do not force the link mode
From: Antoine Tenart @ 2017-08-28 11:32 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Antoine Tenart, Marcin Wojtas, David S. Miller, kishon,
Andrew Lunn, Jason Cooper, Sebastian Hesselbarth,
Gregory Clément, Thomas Petazzoni, nadavh, linux-kernel,
Stefan Chulski, Miquèl Raynal, netdev
In-Reply-To: <20170828110624.GG20805@n2100.armlinux.org.uk>
[-- Attachment #1: Type: text/plain, Size: 2557 bytes --]
On Mon, Aug 28, 2017 at 12:06:24PM +0100, Russell King - ARM Linux wrote:
> On Mon, Aug 28, 2017 at 11:40:51AM +0200, Antoine Tenart wrote:
> > On Mon, Aug 28, 2017 at 09:51:52AM +0100, Russell King - ARM Linux wrote:
> > > On Mon, Aug 28, 2017 at 10:38:37AM +0200, Marcin Wojtas wrote:
> > > >
> > > > Can you be 100% sure that when using SGMII with PHY's (like Marvell
> > > > Alaska 88E1xxx series), is in-band link information always available?
> > > > I'd be very cautious with such assumption and use in-band management
> > > > only when set in the DT, like mvneta. I think phylib can properly can
> > > > do its work when MDIO connection is provided on the board.
> > >
> > > There is another issue to be aware of: if you're wanting to use flow
> > > control autonegotiation, that is not carried across SGMII's in-band
> > > signalling. If you want to use SGMII's in-band signalling for the
> > > duplex and speed information, you still need phylib's notification
> > > to properly set the flow control.
> > >
> > > <plug>
> > > Switching mvpp2 to use phylink (which is needed for the 1G SFP slot on
> > > mcbin) will handle all this for you - dealing with both in-band and
> > > out-of-band negotiation methods, and combining them in the appropriate
> > > manner for the selected operation mode.
> > > </plug>
> >
> > So probably the best move here is to remove this patch, and wait for the
> > phylink support in the PPv2 driver.
>
> I've nothing on that specifically for the mvpp2 driver - what I have is
> for mvneta and the Marvell mvpp2x driver, with GMAC support extracted
> from mvneta (that last bit is rather dirty at the moment so not
> published anywhere, and doesn't cater for PP v2.1 at all.)
>
> I ought to have posted the mvneta part of the phylink patches, but I
> didn't get around to it early enough in this cycle - there are probably
> quite a number of conflicts with net-next now, so I think it's too late
> to submit it for mainline.
>
> I know Andrew has already looked at them in my git tree as part of the
> review of phylink when that was merged - which should be adequate to
> give an example of how to implement it for the mainline PP v2 driver.
OK, good. When looking at the phylink support in the PPv2 driver we'll
look into what you did for mvneta (I saw it as well on your tree). In
the meantime I'll respin this series without this patch.
Thanks!
Antoine
--
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [patch net-next 2/3] net/sched: Change cls_flower to use IDR
From: Simon Horman @ 2017-08-28 11:37 UTC (permalink / raw)
To: Chris Mi; +Cc: netdev, jhs, xiyou.wangcong, jiri, davem, mawilcox
In-Reply-To: <1503902477-39829-3-git-send-email-chrism@mellanox.com>
On Mon, Aug 28, 2017 at 02:41:16AM -0400, Chris Mi wrote:
> Currently, all filters with the same priority are linked in a doubly
> linked list. Every filter should have a unique handle. To make the
> handle unique, we need to iterate the list every time to see if the
> handle exists or not when inserting a new filter. It is time-consuming.
> For example, it takes about 5m3.169s to insert 64K rules.
>
> This patch changes cls_flower to use IDR. With this patch, it
> takes about 0m1.127s to insert 64K rules. The improvement is huge.
Very nice :)
> But please note that in this testing, all filters share the same action.
> If every filter has a unique action, that is another bottleneck.
> Follow-up patch in this patchset addresses that.
>
> Signed-off-by: Chris Mi <chrism@mellanox.com>
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> ---
> net/sched/cls_flower.c | 55 +++++++++++++++++++++-----------------------------
> 1 file changed, 23 insertions(+), 32 deletions(-)
>
> diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
> index bd9dab4..3d041d2 100644
> --- a/net/sched/cls_flower.c
> +++ b/net/sched/cls_flower.c
...
> @@ -890,6 +870,7 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
> struct cls_fl_filter *fnew;
> struct nlattr **tb;
> struct fl_flow_mask mask = {};
> + unsigned long idr_index;
> int err;
>
> if (!tca[TCA_OPTIONS])
> @@ -920,13 +901,21 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
> goto errout;
>
> if (!handle) {
> - handle = fl_grab_new_handle(tp, head);
> - if (!handle) {
> - err = -EINVAL;
> + err = idr_alloc_ext(&head->handle_idr, fnew, &idr_index,
> + 1, 0x80000000, GFP_KERNEL);
> + if (err)
> goto errout;
> - }
> + fnew->handle = idr_index;
> + }
> +
> + /* user specifies a handle and it doesn't exist */
> + if (handle && !fold) {
> + err = idr_alloc_ext(&head->handle_idr, fnew, &idr_index,
> + handle, handle + 1, GFP_KERNEL);
> + if (err)
> + goto errout;
> + fnew->handle = idr_index;
> }
> - fnew->handle = handle;
>
> if (tb[TCA_FLOWER_FLAGS]) {
> fnew->flags = nla_get_u32(tb[TCA_FLOWER_FLAGS]);
> @@ -980,6 +969,8 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
> *arg = fnew;
>
> if (fold) {
> + fnew->handle = handle;
Can it be the case that fold is non-NULL and handle is zero?
The handling of that case seem to have changed in this patch.
> + idr_replace_ext(&head->handle_idr, fnew, fnew->handle);
> list_replace_rcu(&fold->list, &fnew->list);
> tcf_unbind_filter(tp, &fold->res);
> call_rcu(&fold->rcu, fl_destroy_filter);
> --
> 1.8.3.1
>
^ permalink raw reply
* Re: PTP: PHY timestamping when MAC is PTP capable
From: Richard Cochran @ 2017-08-28 12:33 UTC (permalink / raw)
To: Sørensen, Stefan; +Cc: netdev@vger.kernel.org
In-Reply-To: <1503909582.2455.2.camel@spectralink.com>
On Mon, Aug 28, 2017 at 08:39:44AM +0000, Sørensen, Stefan wrote:
> I have run into a problem with packet timestamping on a platform (cpsw
> + dp83640) where both the PHY and the MAC is PTP capable and I need
> the PHY to perform the timestamping. In the current code,
> SIOCGHWTSTAMP is passed to the MAC driver and only if it does not
> support PTP itself will it pass it on to the PHY driver.
>
> I see two ways to fix this:
Just disable CONFIG_CPTS and patch cpsw.c to pass SIOC[GS]HWTSTAMP.
I don't see any reasonable way to make this sort of thing configurable
at run time.
Thanks,
Richard
^ permalink raw reply
* [ethtool] ethtool: Remove UDP Fragmentation Offload use from ethtool
From: Tariq Toukan @ 2017-08-28 12:38 UTC (permalink / raw)
To: John W. Linville; +Cc: netdev, Eran Ben Elisha, Shaker Daibes, Tariq Toukan
From: Shaker Daibes <shakerd@mellanox.com>
UFO was removed in kernel, here we remove it in ethtool app.
Fixes the following issue:
Features for ens8:
Cannot get device udp-fragmentation-offload settings: Operation not supported
Tested with "make check"
Signed-off-by: Shaker Daibes <shakerd@mellanox.com>
Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
---
ethtool.8.in | 3 ---
ethtool.c | 2 --
internal.h | 5 ++---
test-features.c | 36 +++++-------------------------------
4 files changed, 7 insertions(+), 39 deletions(-)
diff --git a/ethtool.8.in b/ethtool.8.in
index 7224b04b8f2f..f14ac53118c8 100644
--- a/ethtool.8.in
+++ b/ethtool.8.in
@@ -480,9 +480,6 @@ Specifies whether scatter-gather should be enabled.
.A2 tso on off
Specifies whether TCP segmentation offload should be enabled.
.TP
-.A2 ufo on off
-Specifies whether UDP fragmentation offload should be enabled
-.TP
.A2 gso on off
Specifies whether generic segmentation offload should be enabled
.TP
diff --git a/ethtool.c b/ethtool.c
index ad18704e7c5f..f3514447c18e 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -166,8 +166,6 @@ static const struct off_flag_def off_flag_def[] = {
ETHTOOL_GSG, ETHTOOL_SSG, ETH_FLAG_SG, 0 },
{ "tso", "tcp-segmentation-offload", "tx-tcp*-segmentation",
ETHTOOL_GTSO, ETHTOOL_STSO, ETH_FLAG_TSO, 0 },
- { "ufo", "udp-fragmentation-offload", "tx-udp-fragmentation",
- ETHTOOL_GUFO, ETHTOOL_SUFO, ETH_FLAG_UFO, 0 },
{ "gso", "generic-segmentation-offload", "tx-generic-segmentation",
ETHTOOL_GGSO, ETHTOOL_SGSO, ETH_FLAG_GSO, 0 },
{ "gro", "generic-receive-offload", "rx-gro",
diff --git a/internal.h b/internal.h
index 4e658ea54307..c033e080e4ff 100644
--- a/internal.h
+++ b/internal.h
@@ -112,12 +112,11 @@ static inline int test_bit(unsigned int nr, const unsigned long *addr)
#define ETH_FLAG_TXCSUM (1 << 1)
#define ETH_FLAG_SG (1 << 2)
#define ETH_FLAG_TSO (1 << 3)
-#define ETH_FLAG_UFO (1 << 4)
#define ETH_FLAG_GSO (1 << 5)
#define ETH_FLAG_GRO (1 << 6)
#define ETH_FLAG_INT_MASK (ETH_FLAG_RXCSUM | ETH_FLAG_TXCSUM | \
- ETH_FLAG_SG | ETH_FLAG_TSO | ETH_FLAG_UFO | \
- ETH_FLAG_GSO | ETH_FLAG_GRO),
+ ETH_FLAG_SG | ETH_FLAG_TSO | ETH_FLAG_GSO | \
+ ETH_FLAG_GRO),
/* Mask of all flags defined for ETHTOOL_{G,S}FLAGS. */
#define ETH_FLAG_EXT_MASK (ETH_FLAG_LRO | ETH_FLAG_RXVLAN | \
ETH_FLAG_TXVLAN | ETH_FLAG_NTUPLE | \
diff --git a/test-features.c b/test-features.c
index 6ebb364803a2..31e2a223d19c 100644
--- a/test-features.c
+++ b/test-features.c
@@ -37,10 +37,6 @@ cmd_gtso_off = { ETHTOOL_GTSO, 0 },
cmd_gtso_on = { ETHTOOL_GTSO, 1 },
cmd_stso_off = { ETHTOOL_STSO, 0 },
cmd_stso_on = { ETHTOOL_STSO, 1 },
-cmd_gufo_off = { ETHTOOL_GUFO, 0 },
-cmd_gufo_on = { ETHTOOL_GUFO, 1 },
-cmd_sufo_off = { ETHTOOL_SUFO, 0 },
-cmd_sufo_on = { ETHTOOL_SUFO, 1 },
cmd_ggso_off = { ETHTOOL_GGSO, 0 },
cmd_ggso_on = { ETHTOOL_GGSO, 1 },
cmd_sgso_off = { ETHTOOL_SGSO, 0 },
@@ -73,7 +69,6 @@ static const struct cmd_expect cmd_expect_get_features_off_old[] = {
{ &cmd_gtxcsum_off, 4, 0, &cmd_gtxcsum_off, sizeof(cmd_gtxcsum_off) },
{ &cmd_gsg_off, 4, 0, &cmd_gsg_off, sizeof(cmd_gsg_off) },
{ &cmd_gtso_off, 4, 0, &cmd_gtso_off, sizeof(cmd_gtso_off) },
- { &cmd_gufo_off, 4, 0, &cmd_gufo_off, sizeof(cmd_gufo_off) },
{ &cmd_ggso_off, 4, 0, &cmd_ggso_off, sizeof(cmd_ggso_off) },
{ &cmd_ggro_off, 4,0, &cmd_ggro_off, sizeof(cmd_ggro_off) },
{ &cmd_gflags_off, 4, 0, &cmd_gflags_off, sizeof(cmd_gflags_off) },
@@ -86,7 +81,6 @@ static const struct cmd_expect cmd_expect_get_features_off_old_some_unsup[] = {
{ &cmd_gtxcsum_off, 4, 0, &cmd_gtxcsum_off, sizeof(cmd_gtxcsum_off) },
{ &cmd_gsg_off, 4, 0, &cmd_gsg_off, sizeof(cmd_gsg_off) },
{ &cmd_gtso_off, 4, 0, &cmd_gtso_off, sizeof(cmd_gtso_off) },
- { &cmd_gufo_off, 4, -EOPNOTSUPP },
{ &cmd_ggso_off, 4, 0, &cmd_ggso_off, sizeof(cmd_ggso_off) },
{ &cmd_ggro_off, 4, -EOPNOTSUPP },
{ &cmd_gflags_off, 4, 0, &cmd_gflags_off, sizeof(cmd_gflags_off) },
@@ -99,7 +93,6 @@ static const struct cmd_expect cmd_expect_get_features_off_old_some_priv[] = {
{ &cmd_gtxcsum_off, 4, 0, &cmd_gtxcsum_off, sizeof(cmd_gtxcsum_off) },
{ &cmd_gsg_off, 4, 0, &cmd_gsg_off, sizeof(cmd_gsg_off) },
{ &cmd_gtso_off, 4, 0, &cmd_gtso_off, sizeof(cmd_gtso_off) },
- { &cmd_gufo_off, 4, 0, &cmd_gufo_off, sizeof(cmd_gufo_off) },
{ &cmd_ggso_off, 4, 0, &cmd_ggso_off, sizeof(cmd_ggso_off) },
{ &cmd_ggro_off, 4, -EPERM },
{ &cmd_gflags_off, 4, 0, &cmd_gflags_off, sizeof(cmd_gflags_off) },
@@ -112,7 +105,6 @@ static const struct cmd_expect cmd_expect_set_features_off_old[] = {
{ &cmd_gtxcsum_on, 4, 0, &cmd_gtxcsum_on, sizeof(cmd_gtxcsum_on) },
{ &cmd_gsg_on, 4, 0, &cmd_gsg_on, sizeof(cmd_gsg_on) },
{ &cmd_gtso_on, 4, 0, &cmd_gtso_on, sizeof(cmd_gtso_on) },
- { &cmd_gufo_on, 4, 0, &cmd_gufo_on, sizeof(cmd_gufo_on) },
{ &cmd_ggso_on, 4, 0, &cmd_ggso_on, sizeof(cmd_ggso_on) },
{ &cmd_ggro_on, 4,0, &cmd_ggro_on, sizeof(cmd_ggro_on) },
{ &cmd_gflags_on, 4, 0, &cmd_gflags_on, sizeof(cmd_sflags_on) },
@@ -120,7 +112,6 @@ static const struct cmd_expect cmd_expect_set_features_off_old[] = {
{ &cmd_stxcsum_off, sizeof(cmd_stxcsum_off), 0, 0, 0 },
{ &cmd_ssg_off, sizeof(cmd_ssg_off), 0, 0, 0 },
{ &cmd_stso_off, sizeof(cmd_stso_off), 0, 0, 0 },
- { &cmd_sufo_off, sizeof(cmd_sufo_off), 0, 0, 0 },
{ &cmd_sgso_off, sizeof(cmd_sgso_off), 0, 0, 0 },
{ &cmd_sgro_off, sizeof(cmd_sgro_off), 0, 0, 0 },
{ &cmd_sflags_off, sizeof(cmd_sflags_off), 0, 0, 0 },
@@ -128,7 +119,6 @@ static const struct cmd_expect cmd_expect_set_features_off_old[] = {
{ &cmd_gtxcsum_off, 4, 0, &cmd_gtxcsum_off, sizeof(cmd_gtxcsum_off) },
{ &cmd_gsg_off, 4, 0, &cmd_gsg_off, sizeof(cmd_gsg_off) },
{ &cmd_gtso_off, 4, 0, &cmd_gtso_off, sizeof(cmd_gtso_off) },
- { &cmd_gufo_off, 4, 0, &cmd_gufo_off, sizeof(cmd_gufo_off) },
{ &cmd_ggso_off, 4, 0, &cmd_ggso_off, sizeof(cmd_ggso_off) },
{ &cmd_ggro_off, 4,0, &cmd_ggro_off, sizeof(cmd_ggro_off) },
{ &cmd_gflags_off, 4, 0, &cmd_gflags_off, sizeof(cmd_sflags_off) },
@@ -141,7 +131,6 @@ static const struct cmd_expect cmd_expect_set_features_on_old[] = {
{ &cmd_gtxcsum_off, 4, 0, &cmd_gtxcsum_off, sizeof(cmd_gtxcsum_off) },
{ &cmd_gsg_off, 4, 0, &cmd_gsg_off, sizeof(cmd_gsg_off) },
{ &cmd_gtso_off, 4, 0, &cmd_gtso_off, sizeof(cmd_gtso_off) },
- { &cmd_gufo_off, 4, 0, &cmd_gufo_off, sizeof(cmd_gufo_off) },
{ &cmd_ggso_off, 4, 0, &cmd_ggso_off, sizeof(cmd_ggso_off) },
{ &cmd_ggro_off, 4,0, &cmd_ggro_off, sizeof(cmd_ggro_off) },
{ &cmd_gflags_off, 4, 0, &cmd_gflags_off, sizeof(cmd_gflags_off) },
@@ -149,7 +138,6 @@ static const struct cmd_expect cmd_expect_set_features_on_old[] = {
{ &cmd_stxcsum_on, sizeof(cmd_stxcsum_on), 0, 0, 0 },
{ &cmd_ssg_on, sizeof(cmd_ssg_on), 0, 0, 0 },
{ &cmd_stso_on, sizeof(cmd_stso_on), 0, 0, 0 },
- { &cmd_sufo_on, sizeof(cmd_sufo_on), 0, 0, 0 },
{ &cmd_sgso_on, sizeof(cmd_sgso_on), 0, 0, 0 },
{ &cmd_sgro_on, sizeof(cmd_sgro_on), 0, 0, 0 },
{ &cmd_sflags_on, sizeof(cmd_sflags_on), 0, 0, 0 },
@@ -157,7 +145,6 @@ static const struct cmd_expect cmd_expect_set_features_on_old[] = {
{ &cmd_gtxcsum_on, 4, 0, &cmd_gtxcsum_on, sizeof(cmd_gtxcsum_on) },
{ &cmd_gsg_on, 4, 0, &cmd_gsg_on, sizeof(cmd_gsg_on) },
{ &cmd_gtso_on, 4, 0, &cmd_gtso_on, sizeof(cmd_gtso_on) },
- { &cmd_gufo_on, 4, 0, &cmd_gufo_on, sizeof(cmd_gufo_on) },
{ &cmd_ggso_on, 4, 0, &cmd_ggso_on, sizeof(cmd_ggso_on) },
{ &cmd_ggro_on, 4,0, &cmd_ggro_on, sizeof(cmd_ggro_on) },
{ &cmd_gflags_on, 4, 0, &cmd_gflags_on, sizeof(cmd_sflags_on) },
@@ -170,7 +157,6 @@ static const struct cmd_expect cmd_expect_set_features_unsup_on_old[] = {
{ &cmd_gtxcsum_off, 4, 0, &cmd_gtxcsum_off, sizeof(cmd_gtxcsum_off) },
{ &cmd_gsg_off, 4, 0, &cmd_gsg_off, sizeof(cmd_gsg_off) },
{ &cmd_gtso_off, 4, 0, &cmd_gtso_off, sizeof(cmd_gtso_off) },
- { &cmd_gufo_off, 4, 0, &cmd_gufo_off, sizeof(cmd_gufo_off) },
{ &cmd_ggso_off, 4, 0, &cmd_ggso_off, sizeof(cmd_ggso_off) },
{ &cmd_ggro_off, 4,0, &cmd_ggro_off, sizeof(cmd_ggro_off) },
{ &cmd_gflags_off, 4, 0, &cmd_gflags_off, sizeof(cmd_gflags_off) },
@@ -290,7 +276,6 @@ static const struct cmd_expect cmd_expect_get_features_min_off[] = {
{ &cmd_gtxcsum_off, 4, 0, &cmd_gtxcsum_off, sizeof(cmd_gtxcsum_off) },
{ &cmd_gsg_off, 4, 0, &cmd_gsg_off, sizeof(cmd_gsg_off) },
{ &cmd_gtso_off, 4, 0, &cmd_gtso_off, sizeof(cmd_gtso_off) },
- { &cmd_gufo_off, 4, 0, &cmd_gufo_off, sizeof(cmd_gufo_off) },
{ &cmd_ggso_off, 4, 0, &cmd_ggso_off, sizeof(cmd_ggso_off) },
{ &cmd_ggro_off, 4,0, &cmd_ggro_off, sizeof(cmd_ggro_off) },
{ &cmd_gflags_off, 4, 0, &cmd_gflags_off, sizeof(cmd_gflags_off) },
@@ -308,7 +293,6 @@ static const struct cmd_expect cmd_expect_get_features_max_on[] = {
{ &cmd_gtxcsum_on, 4, 0, &cmd_gtxcsum_on, sizeof(cmd_gtxcsum_on) },
{ &cmd_gsg_on, 4, 0, &cmd_gsg_on, sizeof(cmd_gsg_on) },
{ &cmd_gtso_on, 4, 0, &cmd_gtso_on, sizeof(cmd_gtso_on) },
- { &cmd_gufo_on, 4, 0, &cmd_gufo_on, sizeof(cmd_gufo_on) },
{ &cmd_ggso_on, 4, 0, &cmd_ggso_on, sizeof(cmd_ggso_on) },
{ &cmd_ggro_on, 4,0, &cmd_ggro_on, sizeof(cmd_ggro_on) },
{ &cmd_gflags_on, 4, 0, &cmd_gflags_on, sizeof(cmd_sflags_on) },
@@ -326,7 +310,6 @@ static const struct cmd_expect cmd_expect_set_features_min_off_min_on[] = {
{ &cmd_gtxcsum_off, 4, 0, &cmd_gtxcsum_off, sizeof(cmd_gtxcsum_off) },
{ &cmd_gsg_off, 4, 0, &cmd_gsg_off, sizeof(cmd_gsg_off) },
{ &cmd_gtso_off, 4, 0, &cmd_gtso_off, sizeof(cmd_gtso_off) },
- { &cmd_gufo_off, 4, 0, &cmd_gufo_off, sizeof(cmd_gufo_off) },
{ &cmd_ggso_off, 4, 0, &cmd_ggso_off, sizeof(cmd_ggso_off) },
{ &cmd_ggro_off, 4,0, &cmd_ggro_off, sizeof(cmd_ggro_off) },
{ &cmd_gflags_off, 4, 0, &cmd_gflags_off, sizeof(cmd_gflags_off) },
@@ -338,7 +321,6 @@ static const struct cmd_expect cmd_expect_set_features_min_off_min_on[] = {
{ &cmd_gtxcsum_off, 4, 0, &cmd_gtxcsum_off, sizeof(cmd_gtxcsum_off) },
{ &cmd_gsg_off, 4, 0, &cmd_gsg_off, sizeof(cmd_gsg_off) },
{ &cmd_gtso_off, 4, 0, &cmd_gtso_off, sizeof(cmd_gtso_off) },
- { &cmd_gufo_off, 4, 0, &cmd_gufo_off, sizeof(cmd_gufo_off) },
{ &cmd_ggso_off, 4, 0, &cmd_ggso_off, sizeof(cmd_ggso_off) },
{ &cmd_ggro_on, 4,0, &cmd_ggro_on, sizeof(cmd_ggro_on) },
{ &cmd_gflags_off, 4, 0, &cmd_gflags_off, sizeof(cmd_gflags_off) },
@@ -356,7 +338,6 @@ static const struct cmd_expect cmd_expect_set_features_min_off_min_off[] = {
{ &cmd_gtxcsum_off, 4, 0, &cmd_gtxcsum_off, sizeof(cmd_gtxcsum_off) },
{ &cmd_gsg_off, 4, 0, &cmd_gsg_off, sizeof(cmd_gsg_off) },
{ &cmd_gtso_off, 4, 0, &cmd_gtso_off, sizeof(cmd_gtso_off) },
- { &cmd_gufo_off, 4, 0, &cmd_gufo_off, sizeof(cmd_gufo_off) },
{ &cmd_ggso_off, 4, 0, &cmd_ggso_off, sizeof(cmd_ggso_off) },
{ &cmd_ggro_off, 4,0, &cmd_ggro_off, sizeof(cmd_ggro_off) },
{ &cmd_gflags_off, 4, 0, &cmd_gflags_off, sizeof(cmd_gflags_off) },
@@ -367,7 +348,6 @@ static const struct cmd_expect cmd_expect_set_features_min_off_min_off[] = {
{ &cmd_gtxcsum_off, 4, 0, &cmd_gtxcsum_off, sizeof(cmd_gtxcsum_off) },
{ &cmd_gsg_off, 4, 0, &cmd_gsg_off, sizeof(cmd_gsg_off) },
{ &cmd_gtso_off, 4, 0, &cmd_gtso_off, sizeof(cmd_gtso_off) },
- { &cmd_gufo_off, 4, 0, &cmd_gufo_off, sizeof(cmd_gufo_off) },
{ &cmd_ggso_off, 4, 0, &cmd_ggso_off, sizeof(cmd_ggso_off) },
{ &cmd_ggro_off, 4,0, &cmd_ggro_off, sizeof(cmd_ggro_off) },
{ &cmd_gflags_off, 4, 0, &cmd_gflags_off, sizeof(cmd_gflags_off) },
@@ -385,7 +365,6 @@ static const struct cmd_expect cmd_expect_set_features_min_on_min_off[] = {
{ &cmd_gtxcsum_off, 4, 0, &cmd_gtxcsum_off, sizeof(cmd_gtxcsum_off) },
{ &cmd_gsg_off, 4, 0, &cmd_gsg_off, sizeof(cmd_gsg_off) },
{ &cmd_gtso_off, 4, 0, &cmd_gtso_off, sizeof(cmd_gtso_off) },
- { &cmd_gufo_off, 4, 0, &cmd_gufo_off, sizeof(cmd_gufo_off) },
{ &cmd_ggso_off, 4, 0, &cmd_ggso_off, sizeof(cmd_ggso_off) },
{ &cmd_ggro_on, 4,0, &cmd_ggro_on, sizeof(cmd_ggro_on) },
{ &cmd_gflags_off, 4, 0, &cmd_gflags_off, sizeof(cmd_gflags_off) },
@@ -396,7 +375,6 @@ static const struct cmd_expect cmd_expect_set_features_min_on_min_off[] = {
{ &cmd_gtxcsum_off, 4, 0, &cmd_gtxcsum_off, sizeof(cmd_gtxcsum_off) },
{ &cmd_gsg_off, 4, 0, &cmd_gsg_off, sizeof(cmd_gsg_off) },
{ &cmd_gtso_off, 4, 0, &cmd_gtso_off, sizeof(cmd_gtso_off) },
- { &cmd_gufo_off, 4, 0, &cmd_gufo_off, sizeof(cmd_gufo_off) },
{ &cmd_ggso_off, 4, 0, &cmd_ggso_off, sizeof(cmd_ggso_off) },
{ &cmd_ggro_off, 4,0, &cmd_ggro_off, sizeof(cmd_ggro_off) },
{ &cmd_gflags_off, 4, 0, &cmd_gflags_off, sizeof(cmd_gflags_off) },
@@ -414,7 +392,6 @@ static const struct cmd_expect cmd_expect_set_features_min_off_unsup_on[] = {
{ &cmd_gtxcsum_off, 4, 0, &cmd_gtxcsum_off, sizeof(cmd_gtxcsum_off) },
{ &cmd_gsg_off, 4, 0, &cmd_gsg_off, sizeof(cmd_gsg_off) },
{ &cmd_gtso_off, 4, 0, &cmd_gtso_off, sizeof(cmd_gtso_off) },
- { &cmd_gufo_off, 4, 0, &cmd_gufo_off, sizeof(cmd_gufo_off) },
{ &cmd_ggso_off, 4, 0, &cmd_ggso_off, sizeof(cmd_ggso_off) },
{ &cmd_ggro_off, 4,0, &cmd_ggro_off, sizeof(cmd_ggro_off) },
{ &cmd_gflags_off, 4, 0, &cmd_gflags_off, sizeof(cmd_gflags_off) },
@@ -425,7 +402,6 @@ static const struct cmd_expect cmd_expect_set_features_min_off_unsup_on[] = {
{ &cmd_gtxcsum_off, 4, 0, &cmd_gtxcsum_off, sizeof(cmd_gtxcsum_off) },
{ &cmd_gsg_off, 4, 0, &cmd_gsg_off, sizeof(cmd_gsg_off) },
{ &cmd_gtso_off, 4, 0, &cmd_gtso_off, sizeof(cmd_gtso_off) },
- { &cmd_gufo_off, 4, 0, &cmd_gufo_off, sizeof(cmd_gufo_off) },
{ &cmd_ggso_off, 4, 0, &cmd_ggso_off, sizeof(cmd_ggso_off) },
{ &cmd_ggro_off, 4,0, &cmd_ggro_off, sizeof(cmd_ggro_off) },
{ &cmd_gflags_off, 4, 0, &cmd_gflags_off, sizeof(cmd_gflags_off) },
@@ -443,7 +419,6 @@ static const struct cmd_expect cmd_expect_set_features_ipv4_off_many_on[] = {
{ &cmd_gtxcsum_off, 4, 0, &cmd_gtxcsum_off, sizeof(cmd_gtxcsum_off) },
{ &cmd_gsg_off, 4, 0, &cmd_gsg_off, sizeof(cmd_gsg_off) },
{ &cmd_gtso_off, 4, 0, &cmd_gtso_off, sizeof(cmd_gtso_off) },
- { &cmd_gufo_off, 4, 0, &cmd_gufo_off, sizeof(cmd_gufo_off) },
{ &cmd_ggso_off, 4, 0, &cmd_ggso_off, sizeof(cmd_ggso_off) },
{ &cmd_ggro_off, 4,0, &cmd_ggro_off, sizeof(cmd_ggro_off) },
{ &cmd_gflags_off, 4, 0, &cmd_gflags_off, sizeof(cmd_gflags_off) },
@@ -454,7 +429,6 @@ static const struct cmd_expect cmd_expect_set_features_ipv4_off_many_on[] = {
{ &cmd_gtxcsum_on, 4, 0, &cmd_gtxcsum_on, sizeof(cmd_gtxcsum_on) },
{ &cmd_gsg_on, 4, 0, &cmd_gsg_on, sizeof(cmd_gsg_on) },
{ &cmd_gtso_on, 4, 0, &cmd_gtso_on, sizeof(cmd_gtso_on) },
- { &cmd_gufo_off, 4, 0, &cmd_gufo_off, sizeof(cmd_gufo_off) },
{ &cmd_ggso_on, 4, 0, &cmd_ggso_on, sizeof(cmd_ggso_on) },
{ &cmd_ggro_on, 4,0, &cmd_ggro_on, sizeof(cmd_ggro_on) },
{ &cmd_gflags_off, 4, 0, &cmd_gflags_off, sizeof(cmd_gflags_off) },
@@ -471,18 +445,18 @@ static struct test_case {
{ 0, "-k devname", cmd_expect_get_features_off_old },
{ 0, "-k dev_unsup", cmd_expect_get_features_off_old_some_unsup },
{ 0, "-k dev_priv", cmd_expect_get_features_off_old_some_priv },
- { 0, "-K devname rx off tx off sg off tso off ufo off gso off lro off rxvlan off txvlan off ntuple off rxhash off gro off",
+ { 0, "-K devname rx off tx off sg off tso off gso off lro off rxvlan off txvlan off ntuple off rxhash off gro off",
cmd_expect_set_features_off_old },
- { 0, "-K devname rx on tx on sg on tso on ufo on gso on lro on rxvlan on txvlan on ntuple on rxhash on gro on",
+ { 0, "-K devname rx on tx on sg on tso on gso on lro on rxvlan on txvlan on ntuple on rxhash on gro on",
cmd_expect_set_features_on_old },
{ 1, "-K devname tx on sg on", cmd_expect_set_features_unsup_on_old },
{ 0, "--show-offload devname", cmd_expect_get_features_min_off },
{ 0, "--show-features devname", cmd_expect_get_features_max_on },
- { 0, "-K devname rx on tx on sg on tso on ufo on gso on gro on",
+ { 0, "-K devname rx on tx on sg on tso on gso on gro on",
cmd_expect_set_features_min_off_min_on },
- { 0, "-K devname rx off tx off sg off tso off ufo off gso off gro off",
+ { 0, "-K devname rx off tx off sg off tso off gso off gro off",
cmd_expect_set_features_min_off_min_off },
- { 0, "-K devname rx off tx off sg off tso off ufo off gso off gro off",
+ { 0, "-K devname rx off tx off sg off tso off gso off gro off",
cmd_expect_set_features_min_on_min_off },
{ 1, "-K devname tx on sg on",
cmd_expect_set_features_min_off_unsup_on },
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH 3/8] tty/bcm63xx_uart: use refclk for the expected clock name
From: Greg Kroah-Hartman @ 2017-08-28 12:51 UTC (permalink / raw)
To: Jonas Gorski
Cc: linux-mips-6z/3iImG2C8G8FEW9MqTrA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-serial-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
Rob Herring, Mark Rutland, Ralf Baechle, Florian Fainelli,
bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w, Kevin Cernekee,
Jiri Slaby, David S. Miller, Russell King
In-Reply-To: <20170802093429.12572-4-jonas.gorski-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On Wed, Aug 02, 2017 at 11:34:24AM +0200, Jonas Gorski wrote:
> We now have the clock available under refclk, so use that.
>
> Signed-off-by: Jonas Gorski <jonas.gorski-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Acked-by: Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH net-next] hinic: don't build the module by default
From: Vitaly Kuznetsov @ 2017-08-28 13:16 UTC (permalink / raw)
To: netdev; +Cc: Aviad Krawczyk, linux-kernel
We probably don't want to enable code supporting particular hardware by
default e.g. when someone does 'make defconfig'. Other ethernet modules
don't do it.
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
drivers/net/ethernet/huawei/hinic/Kconfig | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/ethernet/huawei/hinic/Kconfig b/drivers/net/ethernet/huawei/hinic/Kconfig
index 69f2b1fba48d..08db24954f7e 100644
--- a/drivers/net/ethernet/huawei/hinic/Kconfig
+++ b/drivers/net/ethernet/huawei/hinic/Kconfig
@@ -5,7 +5,6 @@
config HINIC
tristate "Huawei Intelligent PCIE Network Interface Card"
depends on (PCI_MSI && X86)
- default m
---help---
This driver supports HiNIC PCIE Ethernet cards.
To compile this driver as part of the kernel, choose Y here.
--
2.13.5
^ permalink raw reply related
* Re: DSA support for Micrel KSZ8895
From: Andrew Lunn @ 2017-08-28 13:19 UTC (permalink / raw)
To: Pavel Machek
Cc: Woojung.Huh, nathan.leigh.conrad, vivien.didelot, f.fainelli,
netdev, linux-kernel, Tristram.Ha
In-Reply-To: <20170828101451.GC18012@amd>
> I'm working at 4.13-rc.
For network code, it is a good idea to use net-next. That is what you
need to target in order to get patches with new features merged.
Andrew
^ permalink raw reply
* [PATCH net-next 2/4] net/mlx4_core: Make explicit conversion to 64bit value
From: Tariq Toukan @ 2017-08-28 13:38 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Eran Ben Elisha, Leon Romanovsky, Tariq Toukan
In-Reply-To: <1503927503-29065-1-git-send-email-tariqt@mellanox.com>
From: Leon Romanovsky <leonro@mellanox.com>
The "lg" variable is declared as int so in all places where this variable
is used as a shift operand, the output will be int too.
This produces the following smatch warning:
drivers/net/ethernet/mellanox/mlx4/fw.c:1532 mlx4_map_cmd() warn:
should '1 << lg' be a 64 bit type?
Simple declaration of "1" to be "1ULL" will fix the issue.
Fixes: 225c7b1feef1 ("IB/mlx4: Add a driver Mellanox ConnectX InfiniBand adapters")
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx4/fw.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/fw.c b/drivers/net/ethernet/mellanox/mlx4/fw.c
index f76cac01d564..10cfb058a9de 100644
--- a/drivers/net/ethernet/mellanox/mlx4/fw.c
+++ b/drivers/net/ethernet/mellanox/mlx4/fw.c
@@ -1534,7 +1534,7 @@ int mlx4_map_cmd(struct mlx4_dev *dev, u16 op, struct mlx4_icm *icm, u64 virt)
for (i = 0; i < mlx4_icm_size(&iter) >> lg; ++i) {
if (virt != -1) {
pages[nent * 2] = cpu_to_be64(virt);
- virt += 1 << lg;
+ virt += 1ULL << lg;
}
pages[nent * 2 + 1] =
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next 0/4] mlx4 misc patches
From: Tariq Toukan @ 2017-08-28 13:38 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Eran Ben Elisha, Tariq Toukan
Hi Dave,
This patchset contains misc patches from the team
to the mlx4 Core and Eth drivers.
Patch 1 by Eran replaces large static allocations by dynamic ones.
Patch 2 by Leon makes an explicit conversion and solves a smatch warning.
In patch 3 I fix a misplaced brackets of the sizeof operation.
Patch 4 by Moshe adds the ability to inform the FW regarding user mac updates.
Series generated against net-next commit:
901c5d2fbfcd ARM: dts: rk3228-evb: Fix the compiling error
Thanks,
Tariq.
Eran Ben Elisha (1):
net/mlx4_core: Dynamically allocate structs at mlx4_slave_cap
Leon Romanovsky (1):
net/mlx4_core: Make explicit conversion to 64bit value
Moshe Shemesh (1):
net/mlx4: Add user mac FW update support
Tariq Toukan (1):
net/mlx4_core: Fix misplaced brackets of sizeof
drivers/infiniband/hw/mlx4/qp.c | 26 +--
drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 23 ++-
drivers/net/ethernet/mellanox/mlx4/eq.c | 2 +-
drivers/net/ethernet/mellanox/mlx4/fw.c | 19 +-
drivers/net/ethernet/mellanox/mlx4/fw.h | 6 +-
drivers/net/ethernet/mellanox/mlx4/main.c | 238 +++++++++++++------------
drivers/net/ethernet/mellanox/mlx4/mlx4.h | 2 +
drivers/net/ethernet/mellanox/mlx4/port.c | 25 +++
drivers/net/ethernet/mellanox/mlx4/qp.c | 24 +--
include/linux/mlx4/device.h | 16 +-
10 files changed, 219 insertions(+), 162 deletions(-)
--
1.8.3.1
^ permalink raw reply
* [PATCH net-next 4/4] net/mlx4: Add user mac FW update support
From: Tariq Toukan @ 2017-08-28 13:38 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Eran Ben Elisha, Moshe Shemesh, Tariq Toukan
In-Reply-To: <1503927503-29065-1-git-send-email-tariqt@mellanox.com>
From: Moshe Shemesh <moshe@mellanox.com>
Adding support for updating the FW on new port mac, when port mac change
is requested by the user. This info is required by the FW as OEM
management tools require this info directly from the NIC FW.
Check device capability bit to verify the FW supports user mac.
If the FW does support it, use set_port command to notify the FW on the
new mac.
The feature is relevant only to PF port mac.
Signed-off-by: Moshe Shemesh <moshe@mellanox.com>
Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 23 +++++++++++++++++++++--
drivers/net/ethernet/mellanox/mlx4/fw.c | 5 +++++
drivers/net/ethernet/mellanox/mlx4/main.c | 3 +++
drivers/net/ethernet/mellanox/mlx4/mlx4.h | 2 ++
drivers/net/ethernet/mellanox/mlx4/port.c | 25 +++++++++++++++++++++++++
include/linux/mlx4/device.h | 2 ++
6 files changed, 58 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index 6e67ca7aa7f5..3753943aec0f 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -732,6 +732,21 @@ static int mlx4_en_replace_mac(struct mlx4_en_priv *priv, int qpn,
return __mlx4_replace_mac(dev, priv->port, qpn, new_mac_u64);
}
+static void mlx4_en_update_user_mac(struct mlx4_en_priv *priv,
+ unsigned char new_mac[ETH_ALEN + 2])
+{
+ struct mlx4_en_dev *mdev = priv->mdev;
+ int err;
+
+ if (!(mdev->dev->caps.flags2 & MLX4_DEV_CAP_FLAG2_USER_MAC_EN))
+ return;
+
+ err = mlx4_SET_PORT_user_mac(mdev->dev, priv->port, new_mac);
+ if (err)
+ en_err(priv, "Failed to pass user MAC(%pM) to Firmware for port %d, with error %d\n",
+ new_mac, priv->port, err);
+}
+
static int mlx4_en_do_set_mac(struct mlx4_en_priv *priv,
unsigned char new_mac[ETH_ALEN + 2])
{
@@ -766,8 +781,12 @@ static int mlx4_en_set_mac(struct net_device *dev, void *addr)
mutex_lock(&mdev->state_lock);
memcpy(new_mac, saddr->sa_data, ETH_ALEN);
err = mlx4_en_do_set_mac(priv, new_mac);
- if (!err)
- memcpy(dev->dev_addr, saddr->sa_data, ETH_ALEN);
+ if (err)
+ goto out;
+
+ memcpy(dev->dev_addr, saddr->sa_data, ETH_ALEN);
+ mlx4_en_update_user_mac(priv, new_mac);
+out:
mutex_unlock(&mdev->state_lock);
return err;
diff --git a/drivers/net/ethernet/mellanox/mlx4/fw.c b/drivers/net/ethernet/mellanox/mlx4/fw.c
index 10cfb058a9de..d4c7c15cb4ad 100644
--- a/drivers/net/ethernet/mellanox/mlx4/fw.c
+++ b/drivers/net/ethernet/mellanox/mlx4/fw.c
@@ -162,6 +162,7 @@ static void dump_dev_cap_flags2(struct mlx4_dev *dev, u64 flags)
[35] = "Diag counters per port",
[36] = "QinQ VST mode support",
[37] = "sl to vl mapping table change event support",
+ [38] = "user MAC support",
};
int i;
@@ -778,6 +779,7 @@ int mlx4_QUERY_DEV_CAP(struct mlx4_dev *dev, struct mlx4_dev_cap *dev_cap)
#define QUERY_DEV_CAP_MAX_DESC_SZ_SQ_OFFSET 0x52
#define QUERY_DEV_CAP_MAX_SG_RQ_OFFSET 0x55
#define QUERY_DEV_CAP_MAX_DESC_SZ_RQ_OFFSET 0x56
+#define QUERY_DEV_CAP_USER_MAC_EN_OFFSET 0x5C
#define QUERY_DEV_CAP_SVLAN_BY_QP_OFFSET 0x5D
#define QUERY_DEV_CAP_MAX_QP_MCG_OFFSET 0x61
#define QUERY_DEV_CAP_RSVD_MCG_OFFSET 0x62
@@ -949,6 +951,9 @@ int mlx4_QUERY_DEV_CAP(struct mlx4_dev *dev, struct mlx4_dev_cap *dev_cap)
MLX4_GET(size, outbox, QUERY_DEV_CAP_MAX_DESC_SZ_SQ_OFFSET);
dev_cap->max_sq_desc_sz = size;
+ MLX4_GET(field, outbox, QUERY_DEV_CAP_USER_MAC_EN_OFFSET);
+ if (field & (1 << 2))
+ dev_cap->flags2 |= MLX4_DEV_CAP_FLAG2_USER_MAC_EN;
MLX4_GET(field, outbox, QUERY_DEV_CAP_SVLAN_BY_QP_OFFSET);
if (field & 0x1)
dev_cap->flags2 |= MLX4_DEV_CAP_FLAG2_SVLAN_BY_QP;
diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c
index e6413a8a5f07..c631d157b97d 100644
--- a/drivers/net/ethernet/mellanox/mlx4/main.c
+++ b/drivers/net/ethernet/mellanox/mlx4/main.c
@@ -1029,6 +1029,9 @@ static int mlx4_slave_cap(struct mlx4_dev *dev)
dev->caps.flags2 &= ~MLX4_DEV_CAP_FLAG2_TS;
mlx4_warn(dev, "Timestamping is not supported in slave mode\n");
+ dev->caps.flags2 &= ~MLX4_DEV_CAP_FLAG2_USER_MAC_EN;
+ mlx4_dbg(dev, "User MAC FW update is not supported in slave mode\n");
+
slave_adjust_steering_mode(dev, dev_cap, hca_param);
mlx4_dbg(dev, "RSS support for IP fragments is %s\n",
hca_param->rss_ip_frags ? "on" : "off");
diff --git a/drivers/net/ethernet/mellanox/mlx4/mlx4.h b/drivers/net/ethernet/mellanox/mlx4/mlx4.h
index 706d7f21ac5c..16f1e097d33a 100644
--- a/drivers/net/ethernet/mellanox/mlx4/mlx4.h
+++ b/drivers/net/ethernet/mellanox/mlx4/mlx4.h
@@ -807,6 +807,8 @@ struct mlx4_set_port_general_context {
u8 phv_en;
u8 reserved6[5];
__be16 user_mtu;
+ u16 reserved7;
+ u8 user_mac[6];
};
struct mlx4_set_port_rqp_calc_context {
diff --git a/drivers/net/ethernet/mellanox/mlx4/port.c b/drivers/net/ethernet/mellanox/mlx4/port.c
index 4e36e287d605..3ef3406ff4cb 100644
--- a/drivers/net/ethernet/mellanox/mlx4/port.c
+++ b/drivers/net/ethernet/mellanox/mlx4/port.c
@@ -52,6 +52,7 @@
#define MLX4_FLAG2_V_IGNORE_FCS_MASK BIT(1)
#define MLX4_FLAG2_V_USER_MTU_MASK BIT(5)
+#define MLX4_FLAG2_V_USER_MAC_MASK BIT(6)
#define MLX4_FLAG_V_MTU_MASK BIT(0)
#define MLX4_FLAG_V_PPRX_MASK BIT(1)
#define MLX4_FLAG_V_PPTX_MASK BIT(2)
@@ -1700,6 +1701,30 @@ int mlx4_SET_PORT_user_mtu(struct mlx4_dev *dev, u8 port, u16 user_mtu)
}
EXPORT_SYMBOL(mlx4_SET_PORT_user_mtu);
+int mlx4_SET_PORT_user_mac(struct mlx4_dev *dev, u8 port, u8 *user_mac)
+{
+ struct mlx4_cmd_mailbox *mailbox;
+ struct mlx4_set_port_general_context *context;
+ u32 in_mod;
+ int err;
+
+ mailbox = mlx4_alloc_cmd_mailbox(dev);
+ if (IS_ERR(mailbox))
+ return PTR_ERR(mailbox);
+ context = mailbox->buf;
+ context->flags2 |= MLX4_FLAG2_V_USER_MAC_MASK;
+ memcpy(context->user_mac, user_mac, sizeof(context->user_mac));
+
+ in_mod = MLX4_SET_PORT_GENERAL << 8 | port;
+ err = mlx4_cmd(dev, mailbox->dma, in_mod, MLX4_SET_PORT_ETH_OPCODE,
+ MLX4_CMD_SET_PORT, MLX4_CMD_TIME_CLASS_B,
+ MLX4_CMD_NATIVE);
+
+ mlx4_free_cmd_mailbox(dev, mailbox);
+ return err;
+}
+EXPORT_SYMBOL(mlx4_SET_PORT_user_mac);
+
int mlx4_SET_PORT_fcs_check(struct mlx4_dev *dev, u8 port, u8 ignore_fcs_value)
{
struct mlx4_cmd_mailbox *mailbox;
diff --git a/include/linux/mlx4/device.h b/include/linux/mlx4/device.h
index 9844606f9491..89c0e7f7cd9b 100644
--- a/include/linux/mlx4/device.h
+++ b/include/linux/mlx4/device.h
@@ -224,6 +224,7 @@ enum {
MLX4_DEV_CAP_FLAG2_DIAG_PER_PORT = 1ULL << 35,
MLX4_DEV_CAP_FLAG2_SVLAN_BY_QP = 1ULL << 36,
MLX4_DEV_CAP_FLAG2_SL_TO_VL_CHANGE_EVENT = 1ULL << 37,
+ MLX4_DEV_CAP_FLAG2_USER_MAC_EN = 1ULL << 38,
};
enum {
@@ -1377,6 +1378,7 @@ int mlx4_flow_steer_promisc_remove(struct mlx4_dev *dev, u8 port,
int __mlx4_replace_mac(struct mlx4_dev *dev, u8 port, int qpn, u64 new_mac);
int mlx4_SET_PORT_general(struct mlx4_dev *dev, u8 port, int mtu,
u8 pptx, u8 pfctx, u8 pprx, u8 pfcrx);
+int mlx4_SET_PORT_user_mac(struct mlx4_dev *dev, u8 port, u8 *user_mac);
int mlx4_SET_PORT_user_mtu(struct mlx4_dev *dev, u8 port, u16 user_mtu);
int mlx4_SET_PORT_qpn_calc(struct mlx4_dev *dev, u8 port, u32 base_qpn,
u8 promisc);
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next 3/4] net/mlx4_core: Fix misplaced brackets of sizeof
From: Tariq Toukan @ 2017-08-28 13:38 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Eran Ben Elisha, Tariq Toukan, Stephen Hemminger
In-Reply-To: <1503927503-29065-1-git-send-email-tariqt@mellanox.com>
When changing the sizeof style usage in the patch cited below,
one brackets misplacement was introduced. Here we fix it.
Fixes: 31975e27a4b5 ("mlx4: sizeof style usage")
Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
Cc: Stephen Hemminger <stephen@networkplumber.org>
---
drivers/net/ethernet/mellanox/mlx4/eq.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/eq.c b/drivers/net/ethernet/mellanox/mlx4/eq.c
index b98698bf75dd..6f57c052053e 100644
--- a/drivers/net/ethernet/mellanox/mlx4/eq.c
+++ b/drivers/net/ethernet/mellanox/mlx4/eq.c
@@ -726,7 +726,7 @@ static int mlx4_eq_int(struct mlx4_dev *dev, struct mlx4_eq *eq)
}
memcpy(&priv->mfunc.master.comm_arm_bit_vector,
eqe->event.comm_channel_arm.bit_vec,
- sizeof(eqe)->event.comm_channel_arm.bit_vec);
+ sizeof(eqe->event.comm_channel_arm.bit_vec));
queue_work(priv->mfunc.master.comm_wq,
&priv->mfunc.master.comm_work);
break;
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next 1/4] net/mlx4_core: Dynamically allocate structs at mlx4_slave_cap
From: Tariq Toukan @ 2017-08-28 13:38 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, Eran Ben Elisha, Tal Alon, Tariq Toukan, Saeed Mahameed
In-Reply-To: <1503927503-29065-1-git-send-email-tariqt@mellanox.com>
From: Eran Ben Elisha <eranbe@mellanox.com>
In order to avoid temporary large structs on the stack,
allocate them dynamically.
Signed-off-by: Eran Ben Elisha <eranbe@mellanox.com>
Signed-off-by: Tal Alon <talal@mellanox.com>
Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
drivers/infiniband/hw/mlx4/qp.c | 26 ++--
drivers/net/ethernet/mellanox/mlx4/fw.c | 12 +-
drivers/net/ethernet/mellanox/mlx4/fw.h | 6 +-
drivers/net/ethernet/mellanox/mlx4/main.c | 235 ++++++++++++++++--------------
drivers/net/ethernet/mellanox/mlx4/qp.c | 24 +--
include/linux/mlx4/device.h | 14 +-
6 files changed, 159 insertions(+), 158 deletions(-)
diff --git a/drivers/infiniband/hw/mlx4/qp.c b/drivers/infiniband/hw/mlx4/qp.c
index 75c0e6c5dd56..83e5c72f19b8 100644
--- a/drivers/infiniband/hw/mlx4/qp.c
+++ b/drivers/infiniband/hw/mlx4/qp.c
@@ -145,8 +145,8 @@ static int is_sqp(struct mlx4_ib_dev *dev, struct mlx4_ib_qp *qp)
/* VF or PF -- proxy SQP */
if (mlx4_is_mfunc(dev->dev)) {
for (i = 0; i < dev->dev->caps.num_ports; i++) {
- if (qp->mqp.qpn == dev->dev->caps.qp0_proxy[i] ||
- qp->mqp.qpn == dev->dev->caps.qp1_proxy[i]) {
+ if (qp->mqp.qpn == dev->dev->caps.spec_qps[i].qp0_proxy ||
+ qp->mqp.qpn == dev->dev->caps.spec_qps[i].qp1_proxy) {
proxy_sqp = 1;
break;
}
@@ -173,7 +173,7 @@ static int is_qp0(struct mlx4_ib_dev *dev, struct mlx4_ib_qp *qp)
/* VF or PF -- proxy QP0 */
if (mlx4_is_mfunc(dev->dev)) {
for (i = 0; i < dev->dev->caps.num_ports; i++) {
- if (qp->mqp.qpn == dev->dev->caps.qp0_proxy[i]) {
+ if (qp->mqp.qpn == dev->dev->caps.spec_qps[i].qp0_proxy) {
proxy_qp0 = 1;
break;
}
@@ -614,8 +614,8 @@ static int qp0_enabled_vf(struct mlx4_dev *dev, int qpn)
{
int i;
for (i = 0; i < dev->caps.num_ports; i++) {
- if (qpn == dev->caps.qp0_proxy[i])
- return !!dev->caps.qp0_qkey[i];
+ if (qpn == dev->caps.spec_qps[i].qp0_proxy)
+ return !!dev->caps.spec_qps[i].qp0_qkey;
}
return 0;
}
@@ -1114,9 +1114,9 @@ static u32 get_sqp_num(struct mlx4_ib_dev *dev, struct ib_qp_init_attr *attr)
}
/* PF or VF -- creating proxies */
if (attr->qp_type == IB_QPT_SMI)
- return dev->dev->caps.qp0_proxy[attr->port_num - 1];
+ return dev->dev->caps.spec_qps[attr->port_num - 1].qp0_proxy;
else
- return dev->dev->caps.qp1_proxy[attr->port_num - 1];
+ return dev->dev->caps.spec_qps[attr->port_num - 1].qp1_proxy;
}
static struct ib_qp *_mlx4_ib_create_qp(struct ib_pd *pd,
@@ -2277,9 +2277,9 @@ static int vf_get_qp0_qkey(struct mlx4_dev *dev, int qpn, u32 *qkey)
{
int i;
for (i = 0; i < dev->caps.num_ports; i++) {
- if (qpn == dev->caps.qp0_proxy[i] ||
- qpn == dev->caps.qp0_tunnel[i]) {
- *qkey = dev->caps.qp0_qkey[i];
+ if (qpn == dev->caps.spec_qps[i].qp0_proxy ||
+ qpn == dev->caps.spec_qps[i].qp0_tunnel) {
+ *qkey = dev->caps.spec_qps[i].qp0_qkey;
return 0;
}
}
@@ -2340,7 +2340,7 @@ static int build_sriov_qp0_header(struct mlx4_ib_sqp *sqp,
sqp->ud_header.bth.destination_qpn = cpu_to_be32(wr->remote_qpn);
else
sqp->ud_header.bth.destination_qpn =
- cpu_to_be32(mdev->dev->caps.qp0_tunnel[sqp->qp.port - 1]);
+ cpu_to_be32(mdev->dev->caps.spec_qps[sqp->qp.port - 1].qp0_tunnel);
sqp->ud_header.bth.psn = cpu_to_be32((sqp->send_psn++) & ((1 << 24) - 1));
if (mlx4_is_master(mdev->dev)) {
@@ -2800,9 +2800,9 @@ static void set_tunnel_datagram_seg(struct mlx4_ib_dev *dev,
memcpy(dseg->av, &sqp_av, sizeof (struct mlx4_av));
if (qpt == MLX4_IB_QPT_PROXY_GSI)
- dseg->dqpn = cpu_to_be32(dev->dev->caps.qp1_tunnel[port - 1]);
+ dseg->dqpn = cpu_to_be32(dev->dev->caps.spec_qps[port - 1].qp1_tunnel);
else
- dseg->dqpn = cpu_to_be32(dev->dev->caps.qp0_tunnel[port - 1]);
+ dseg->dqpn = cpu_to_be32(dev->dev->caps.spec_qps[port - 1].qp0_tunnel);
/* Use QKEY from the QP context, which is set by master */
dseg->qkey = cpu_to_be32(IB_QP_SET_QKEY);
}
diff --git a/drivers/net/ethernet/mellanox/mlx4/fw.c b/drivers/net/ethernet/mellanox/mlx4/fw.c
index 042707623922..f76cac01d564 100644
--- a/drivers/net/ethernet/mellanox/mlx4/fw.c
+++ b/drivers/net/ethernet/mellanox/mlx4/fw.c
@@ -679,22 +679,22 @@ int mlx4_QUERY_FUNC_CAP(struct mlx4_dev *dev, u8 gen_or_port,
if (func_cap->flags1 & QUERY_FUNC_CAP_VF_ENABLE_QP0) {
MLX4_GET(qkey, outbox, QUERY_FUNC_CAP_PRIV_VF_QKEY_OFFSET);
- func_cap->qp0_qkey = qkey;
+ func_cap->spec_qps.qp0_qkey = qkey;
} else {
- func_cap->qp0_qkey = 0;
+ func_cap->spec_qps.qp0_qkey = 0;
}
MLX4_GET(size, outbox, QUERY_FUNC_CAP_QP0_TUNNEL);
- func_cap->qp0_tunnel_qpn = size & 0xFFFFFF;
+ func_cap->spec_qps.qp0_tunnel = size & 0xFFFFFF;
MLX4_GET(size, outbox, QUERY_FUNC_CAP_QP0_PROXY);
- func_cap->qp0_proxy_qpn = size & 0xFFFFFF;
+ func_cap->spec_qps.qp0_proxy = size & 0xFFFFFF;
MLX4_GET(size, outbox, QUERY_FUNC_CAP_QP1_TUNNEL);
- func_cap->qp1_tunnel_qpn = size & 0xFFFFFF;
+ func_cap->spec_qps.qp1_tunnel = size & 0xFFFFFF;
MLX4_GET(size, outbox, QUERY_FUNC_CAP_QP1_PROXY);
- func_cap->qp1_proxy_qpn = size & 0xFFFFFF;
+ func_cap->spec_qps.qp1_proxy = size & 0xFFFFFF;
if (func_cap->flags1 & QUERY_FUNC_CAP_FLAGS1_NIC_INFO)
MLX4_GET(func_cap->phys_port_id, outbox,
diff --git a/drivers/net/ethernet/mellanox/mlx4/fw.h b/drivers/net/ethernet/mellanox/mlx4/fw.h
index b52ba01aa486..cd6399c76bfd 100644
--- a/drivers/net/ethernet/mellanox/mlx4/fw.h
+++ b/drivers/net/ethernet/mellanox/mlx4/fw.h
@@ -144,11 +144,7 @@ struct mlx4_func_cap {
int max_eq;
int reserved_eq;
int mcg_quota;
- u32 qp0_qkey;
- u32 qp0_tunnel_qpn;
- u32 qp0_proxy_qpn;
- u32 qp1_tunnel_qpn;
- u32 qp1_proxy_qpn;
+ struct mlx4_spec_qps spec_qps;
u32 reserved_lkey;
u8 physical_port;
u8 flags0;
diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c
index 8404e165eca4..e6413a8a5f07 100644
--- a/drivers/net/ethernet/mellanox/mlx4/main.c
+++ b/drivers/net/ethernet/mellanox/mlx4/main.c
@@ -819,38 +819,93 @@ static void slave_adjust_steering_mode(struct mlx4_dev *dev,
mlx4_steering_mode_str(dev->caps.steering_mode));
}
+static void mlx4_slave_destroy_special_qp_cap(struct mlx4_dev *dev)
+{
+ kfree(dev->caps.spec_qps);
+ dev->caps.spec_qps = NULL;
+}
+
+static int mlx4_slave_special_qp_cap(struct mlx4_dev *dev)
+{
+ struct mlx4_func_cap *func_cap = NULL;
+ struct mlx4_caps *caps = &dev->caps;
+ int i, err = 0;
+
+ func_cap = kzalloc(sizeof(*func_cap), GFP_KERNEL);
+ caps->spec_qps = kcalloc(caps->num_ports, sizeof(*caps->spec_qps), GFP_KERNEL);
+
+ if (!func_cap || !caps->spec_qps) {
+ mlx4_err(dev, "Failed to allocate memory for special qps cap\n");
+ err = -ENOMEM;
+ goto err_mem;
+ }
+
+ for (i = 1; i <= caps->num_ports; ++i) {
+ err = mlx4_QUERY_FUNC_CAP(dev, i, func_cap);
+ if (err) {
+ mlx4_err(dev, "QUERY_FUNC_CAP port command failed for port %d, aborting (%d)\n",
+ i, err);
+ goto err_mem;
+ }
+ caps->spec_qps[i - 1] = func_cap->spec_qps;
+ caps->port_mask[i] = caps->port_type[i];
+ caps->phys_port_id[i] = func_cap->phys_port_id;
+ err = mlx4_get_slave_pkey_gid_tbl_len(dev, i,
+ &caps->gid_table_len[i],
+ &caps->pkey_table_len[i]);
+ if (err) {
+ mlx4_err(dev, "QUERY_PORT command failed for port %d, aborting (%d)\n",
+ i, err);
+ goto err_mem;
+ }
+ }
+
+err_mem:
+ if (err)
+ mlx4_slave_destroy_special_qp_cap(dev);
+ kfree(func_cap);
+ return err;
+}
+
static int mlx4_slave_cap(struct mlx4_dev *dev)
{
int err;
u32 page_size;
- struct mlx4_dev_cap dev_cap;
- struct mlx4_func_cap func_cap;
- struct mlx4_init_hca_param hca_param;
- u8 i;
+ struct mlx4_dev_cap *dev_cap = NULL;
+ struct mlx4_func_cap *func_cap = NULL;
+ struct mlx4_init_hca_param *hca_param = NULL;
+
+ hca_param = kzalloc(sizeof(*hca_param), GFP_KERNEL);
+ func_cap = kzalloc(sizeof(*func_cap), GFP_KERNEL);
+ dev_cap = kzalloc(sizeof(*dev_cap), GFP_KERNEL);
+ if (!hca_param || !func_cap || !dev_cap) {
+ mlx4_err(dev, "Failed to allocate memory for slave_cap\n");
+ err = -ENOMEM;
+ goto free_mem;
+ }
- memset(&hca_param, 0, sizeof(hca_param));
- err = mlx4_QUERY_HCA(dev, &hca_param);
+ err = mlx4_QUERY_HCA(dev, hca_param);
if (err) {
mlx4_err(dev, "QUERY_HCA command failed, aborting\n");
- return err;
+ goto free_mem;
}
/* fail if the hca has an unknown global capability
* at this time global_caps should be always zeroed
*/
- if (hca_param.global_caps) {
+ if (hca_param->global_caps) {
mlx4_err(dev, "Unknown hca global capabilities\n");
- return -EINVAL;
+ err = -EINVAL;
+ goto free_mem;
}
- dev->caps.hca_core_clock = hca_param.hca_core_clock;
+ dev->caps.hca_core_clock = hca_param->hca_core_clock;
- memset(&dev_cap, 0, sizeof(dev_cap));
- dev->caps.max_qp_dest_rdma = 1 << hca_param.log_rd_per_qp;
- err = mlx4_dev_cap(dev, &dev_cap);
+ dev->caps.max_qp_dest_rdma = 1 << hca_param->log_rd_per_qp;
+ err = mlx4_dev_cap(dev, dev_cap);
if (err) {
mlx4_err(dev, "QUERY_DEV_CAP command failed, aborting\n");
- return err;
+ goto free_mem;
}
err = mlx4_QUERY_FW(dev);
@@ -862,21 +917,23 @@ static int mlx4_slave_cap(struct mlx4_dev *dev)
if (page_size > PAGE_SIZE) {
mlx4_err(dev, "HCA minimum page size of %d bigger than kernel PAGE_SIZE of %ld, aborting\n",
page_size, PAGE_SIZE);
- return -ENODEV;
+ err = -ENODEV;
+ goto free_mem;
}
/* Set uar_page_shift for VF */
- dev->uar_page_shift = hca_param.uar_page_sz + 12;
+ dev->uar_page_shift = hca_param->uar_page_sz + 12;
/* Make sure the master uar page size is valid */
if (dev->uar_page_shift > PAGE_SHIFT) {
mlx4_err(dev,
"Invalid configuration: uar page size is larger than system page size\n");
- return -ENODEV;
+ err = -ENODEV;
+ goto free_mem;
}
/* Set reserved_uars based on the uar_page_shift */
- mlx4_set_num_reserved_uars(dev, &dev_cap);
+ mlx4_set_num_reserved_uars(dev, dev_cap);
/* Although uar page size in FW differs from system page size,
* upper software layers (mlx4_ib, mlx4_en and part of mlx4_core)
@@ -884,34 +941,35 @@ static int mlx4_slave_cap(struct mlx4_dev *dev)
*/
dev->caps.uar_page_size = PAGE_SIZE;
- memset(&func_cap, 0, sizeof(func_cap));
- err = mlx4_QUERY_FUNC_CAP(dev, 0, &func_cap);
+ err = mlx4_QUERY_FUNC_CAP(dev, 0, func_cap);
if (err) {
mlx4_err(dev, "QUERY_FUNC_CAP general command failed, aborting (%d)\n",
err);
- return err;
+ goto free_mem;
}
- if ((func_cap.pf_context_behaviour | PF_CONTEXT_BEHAVIOUR_MASK) !=
+ if ((func_cap->pf_context_behaviour | PF_CONTEXT_BEHAVIOUR_MASK) !=
PF_CONTEXT_BEHAVIOUR_MASK) {
mlx4_err(dev, "Unknown pf context behaviour %x known flags %x\n",
- func_cap.pf_context_behaviour, PF_CONTEXT_BEHAVIOUR_MASK);
- return -EINVAL;
- }
-
- dev->caps.num_ports = func_cap.num_ports;
- dev->quotas.qp = func_cap.qp_quota;
- dev->quotas.srq = func_cap.srq_quota;
- dev->quotas.cq = func_cap.cq_quota;
- dev->quotas.mpt = func_cap.mpt_quota;
- dev->quotas.mtt = func_cap.mtt_quota;
- dev->caps.num_qps = 1 << hca_param.log_num_qps;
- dev->caps.num_srqs = 1 << hca_param.log_num_srqs;
- dev->caps.num_cqs = 1 << hca_param.log_num_cqs;
- dev->caps.num_mpts = 1 << hca_param.log_mpt_sz;
- dev->caps.num_eqs = func_cap.max_eq;
- dev->caps.reserved_eqs = func_cap.reserved_eq;
- dev->caps.reserved_lkey = func_cap.reserved_lkey;
+ func_cap->pf_context_behaviour,
+ PF_CONTEXT_BEHAVIOUR_MASK);
+ err = -EINVAL;
+ goto free_mem;
+ }
+
+ dev->caps.num_ports = func_cap->num_ports;
+ dev->quotas.qp = func_cap->qp_quota;
+ dev->quotas.srq = func_cap->srq_quota;
+ dev->quotas.cq = func_cap->cq_quota;
+ dev->quotas.mpt = func_cap->mpt_quota;
+ dev->quotas.mtt = func_cap->mtt_quota;
+ dev->caps.num_qps = 1 << hca_param->log_num_qps;
+ dev->caps.num_srqs = 1 << hca_param->log_num_srqs;
+ dev->caps.num_cqs = 1 << hca_param->log_num_cqs;
+ dev->caps.num_mpts = 1 << hca_param->log_mpt_sz;
+ dev->caps.num_eqs = func_cap->max_eq;
+ dev->caps.reserved_eqs = func_cap->reserved_eq;
+ dev->caps.reserved_lkey = func_cap->reserved_lkey;
dev->caps.num_pds = MLX4_NUM_PDS;
dev->caps.num_mgms = 0;
dev->caps.num_amgms = 0;
@@ -924,38 +982,10 @@ static int mlx4_slave_cap(struct mlx4_dev *dev)
mlx4_replace_zero_macs(dev);
- dev->caps.qp0_qkey = kcalloc(dev->caps.num_ports, sizeof(u32), GFP_KERNEL);
- dev->caps.qp0_tunnel = kcalloc(dev->caps.num_ports, sizeof(u32), GFP_KERNEL);
- dev->caps.qp0_proxy = kcalloc(dev->caps.num_ports, sizeof(u32), GFP_KERNEL);
- dev->caps.qp1_tunnel = kcalloc(dev->caps.num_ports, sizeof(u32), GFP_KERNEL);
- dev->caps.qp1_proxy = kcalloc(dev->caps.num_ports, sizeof(u32), GFP_KERNEL);
-
- if (!dev->caps.qp0_tunnel || !dev->caps.qp0_proxy ||
- !dev->caps.qp1_tunnel || !dev->caps.qp1_proxy ||
- !dev->caps.qp0_qkey) {
- err = -ENOMEM;
- goto err_mem;
- }
-
- for (i = 1; i <= dev->caps.num_ports; ++i) {
- err = mlx4_QUERY_FUNC_CAP(dev, i, &func_cap);
- if (err) {
- mlx4_err(dev, "QUERY_FUNC_CAP port command failed for port %d, aborting (%d)\n",
- i, err);
- goto err_mem;
- }
- dev->caps.qp0_qkey[i - 1] = func_cap.qp0_qkey;
- dev->caps.qp0_tunnel[i - 1] = func_cap.qp0_tunnel_qpn;
- dev->caps.qp0_proxy[i - 1] = func_cap.qp0_proxy_qpn;
- dev->caps.qp1_tunnel[i - 1] = func_cap.qp1_tunnel_qpn;
- dev->caps.qp1_proxy[i - 1] = func_cap.qp1_proxy_qpn;
- dev->caps.port_mask[i] = dev->caps.port_type[i];
- dev->caps.phys_port_id[i] = func_cap.phys_port_id;
- err = mlx4_get_slave_pkey_gid_tbl_len(dev, i,
- &dev->caps.gid_table_len[i],
- &dev->caps.pkey_table_len[i]);
- if (err)
- goto err_mem;
+ err = mlx4_slave_special_qp_cap(dev);
+ if (err) {
+ mlx4_err(dev, "Set special QP caps failed. aborting\n");
+ goto free_mem;
}
if (dev->caps.uar_page_size * (dev->caps.num_uars -
@@ -970,7 +1000,7 @@ static int mlx4_slave_cap(struct mlx4_dev *dev)
goto err_mem;
}
- if (hca_param.dev_cap_enabled & MLX4_DEV_CAP_64B_EQE_ENABLED) {
+ if (hca_param->dev_cap_enabled & MLX4_DEV_CAP_64B_EQE_ENABLED) {
dev->caps.eqe_size = 64;
dev->caps.eqe_factor = 1;
} else {
@@ -978,20 +1008,20 @@ static int mlx4_slave_cap(struct mlx4_dev *dev)
dev->caps.eqe_factor = 0;
}
- if (hca_param.dev_cap_enabled & MLX4_DEV_CAP_64B_CQE_ENABLED) {
+ if (hca_param->dev_cap_enabled & MLX4_DEV_CAP_64B_CQE_ENABLED) {
dev->caps.cqe_size = 64;
dev->caps.userspace_caps |= MLX4_USER_DEV_CAP_LARGE_CQE;
} else {
dev->caps.cqe_size = 32;
}
- if (hca_param.dev_cap_enabled & MLX4_DEV_CAP_EQE_STRIDE_ENABLED) {
- dev->caps.eqe_size = hca_param.eqe_size;
+ if (hca_param->dev_cap_enabled & MLX4_DEV_CAP_EQE_STRIDE_ENABLED) {
+ dev->caps.eqe_size = hca_param->eqe_size;
dev->caps.eqe_factor = 0;
}
- if (hca_param.dev_cap_enabled & MLX4_DEV_CAP_CQE_STRIDE_ENABLED) {
- dev->caps.cqe_size = hca_param.cqe_size;
+ if (hca_param->dev_cap_enabled & MLX4_DEV_CAP_CQE_STRIDE_ENABLED) {
+ dev->caps.cqe_size = hca_param->cqe_size;
/* User still need to know when CQE > 32B */
dev->caps.userspace_caps |= MLX4_USER_DEV_CAP_LARGE_CQE;
}
@@ -999,31 +1029,24 @@ static int mlx4_slave_cap(struct mlx4_dev *dev)
dev->caps.flags2 &= ~MLX4_DEV_CAP_FLAG2_TS;
mlx4_warn(dev, "Timestamping is not supported in slave mode\n");
- slave_adjust_steering_mode(dev, &dev_cap, &hca_param);
+ slave_adjust_steering_mode(dev, dev_cap, hca_param);
mlx4_dbg(dev, "RSS support for IP fragments is %s\n",
- hca_param.rss_ip_frags ? "on" : "off");
+ hca_param->rss_ip_frags ? "on" : "off");
- if (func_cap.extra_flags & MLX4_QUERY_FUNC_FLAGS_BF_RES_QP &&
+ if (func_cap->extra_flags & MLX4_QUERY_FUNC_FLAGS_BF_RES_QP &&
dev->caps.bf_reg_size)
dev->caps.alloc_res_qp_mask |= MLX4_RESERVE_ETH_BF_QP;
- if (func_cap.extra_flags & MLX4_QUERY_FUNC_FLAGS_A0_RES_QP)
+ if (func_cap->extra_flags & MLX4_QUERY_FUNC_FLAGS_A0_RES_QP)
dev->caps.alloc_res_qp_mask |= MLX4_RESERVE_A0_QP;
- return 0;
-
err_mem:
- kfree(dev->caps.qp0_qkey);
- kfree(dev->caps.qp0_tunnel);
- kfree(dev->caps.qp0_proxy);
- kfree(dev->caps.qp1_tunnel);
- kfree(dev->caps.qp1_proxy);
- dev->caps.qp0_qkey = NULL;
- dev->caps.qp0_tunnel = NULL;
- dev->caps.qp0_proxy = NULL;
- dev->caps.qp1_tunnel = NULL;
- dev->caps.qp1_proxy = NULL;
-
+ if (err)
+ mlx4_slave_destroy_special_qp_cap(dev);
+free_mem:
+ kfree(hca_param);
+ kfree(func_cap);
+ kfree(dev_cap);
return err;
}
@@ -2407,13 +2430,8 @@ static int mlx4_init_hca(struct mlx4_dev *dev)
unmap_internal_clock(dev);
unmap_bf_area(dev);
- if (mlx4_is_slave(dev)) {
- kfree(dev->caps.qp0_qkey);
- kfree(dev->caps.qp0_tunnel);
- kfree(dev->caps.qp0_proxy);
- kfree(dev->caps.qp1_tunnel);
- kfree(dev->caps.qp1_proxy);
- }
+ if (mlx4_is_slave(dev))
+ mlx4_slave_destroy_special_qp_cap(dev);
err_close:
if (mlx4_is_slave(dev))
@@ -3596,13 +3614,8 @@ static int mlx4_load_one(struct pci_dev *pdev, int pci_dev_data,
mlx4_multi_func_cleanup(dev);
}
- if (mlx4_is_slave(dev)) {
- kfree(dev->caps.qp0_qkey);
- kfree(dev->caps.qp0_tunnel);
- kfree(dev->caps.qp0_proxy);
- kfree(dev->caps.qp1_tunnel);
- kfree(dev->caps.qp1_proxy);
- }
+ if (mlx4_is_slave(dev))
+ mlx4_slave_destroy_special_qp_cap(dev);
err_close:
mlx4_close_hca(dev);
@@ -3942,11 +3955,7 @@ static void mlx4_unload_one(struct pci_dev *pdev)
if (!mlx4_is_slave(dev))
mlx4_free_ownership(dev);
- kfree(dev->caps.qp0_qkey);
- kfree(dev->caps.qp0_tunnel);
- kfree(dev->caps.qp0_proxy);
- kfree(dev->caps.qp1_tunnel);
- kfree(dev->caps.qp1_proxy);
+ mlx4_slave_destroy_special_qp_cap(dev);
kfree(dev->dev_vfs);
mlx4_clean_dev(dev);
diff --git a/drivers/net/ethernet/mellanox/mlx4/qp.c b/drivers/net/ethernet/mellanox/mlx4/qp.c
index 2b067763a6bc..28c441c0d31f 100644
--- a/drivers/net/ethernet/mellanox/mlx4/qp.c
+++ b/drivers/net/ethernet/mellanox/mlx4/qp.c
@@ -844,24 +844,20 @@ int mlx4_init_qp_table(struct mlx4_dev *dev)
/* In mfunc, calculate proxy and tunnel qp offsets for the PF here,
* since the PF does not call mlx4_slave_caps */
- dev->caps.qp0_tunnel = kcalloc(dev->caps.num_ports, sizeof(u32), GFP_KERNEL);
- dev->caps.qp0_proxy = kcalloc(dev->caps.num_ports, sizeof(u32), GFP_KERNEL);
- dev->caps.qp1_tunnel = kcalloc(dev->caps.num_ports, sizeof(u32), GFP_KERNEL);
- dev->caps.qp1_proxy = kcalloc(dev->caps.num_ports, sizeof(u32), GFP_KERNEL);
+ dev->caps.spec_qps = kcalloc(dev->caps.num_ports, sizeof(dev->caps.spec_qps), GFP_KERNEL);
- if (!dev->caps.qp0_tunnel || !dev->caps.qp0_proxy ||
- !dev->caps.qp1_tunnel || !dev->caps.qp1_proxy) {
+ if (!dev->caps.spec_qps) {
err = -ENOMEM;
goto err_mem;
}
for (k = 0; k < dev->caps.num_ports; k++) {
- dev->caps.qp0_proxy[k] = dev->phys_caps.base_proxy_sqpn +
+ dev->caps.spec_qps[k].qp0_proxy = dev->phys_caps.base_proxy_sqpn +
8 * mlx4_master_func_num(dev) + k;
- dev->caps.qp0_tunnel[k] = dev->caps.qp0_proxy[k] + 8 * MLX4_MFUNC_MAX;
- dev->caps.qp1_proxy[k] = dev->phys_caps.base_proxy_sqpn +
+ dev->caps.spec_qps[k].qp0_tunnel = dev->caps.spec_qps[k].qp0_proxy + 8 * MLX4_MFUNC_MAX;
+ dev->caps.spec_qps[k].qp1_proxy = dev->phys_caps.base_proxy_sqpn +
8 * mlx4_master_func_num(dev) + MLX4_MAX_PORTS + k;
- dev->caps.qp1_tunnel[k] = dev->caps.qp1_proxy[k] + 8 * MLX4_MFUNC_MAX;
+ dev->caps.spec_qps[k].qp1_tunnel = dev->caps.spec_qps[k].qp1_proxy + 8 * MLX4_MFUNC_MAX;
}
}
@@ -873,12 +869,8 @@ int mlx4_init_qp_table(struct mlx4_dev *dev)
return err;
err_mem:
- kfree(dev->caps.qp0_tunnel);
- kfree(dev->caps.qp0_proxy);
- kfree(dev->caps.qp1_tunnel);
- kfree(dev->caps.qp1_proxy);
- dev->caps.qp0_tunnel = dev->caps.qp0_proxy =
- dev->caps.qp1_tunnel = dev->caps.qp1_proxy = NULL;
+ kfree(dev->caps.spec_qps);
+ dev->caps.spec_qps = NULL;
mlx4_cleanup_qp_zones(dev);
return err;
}
diff --git a/include/linux/mlx4/device.h b/include/linux/mlx4/device.h
index b54517c05e9a..9844606f9491 100644
--- a/include/linux/mlx4/device.h
+++ b/include/linux/mlx4/device.h
@@ -518,6 +518,14 @@ struct mlx4_phys_caps {
u32 base_tunnel_sqpn;
};
+struct mlx4_spec_qps {
+ u32 qp0_qkey;
+ u32 qp0_proxy;
+ u32 qp0_tunnel;
+ u32 qp1_proxy;
+ u32 qp1_tunnel;
+};
+
struct mlx4_caps {
u64 fw_ver;
u32 function;
@@ -547,11 +555,7 @@ struct mlx4_caps {
int max_qp_init_rdma;
int max_qp_dest_rdma;
int max_tc_eth;
- u32 *qp0_qkey;
- u32 *qp0_proxy;
- u32 *qp1_proxy;
- u32 *qp0_tunnel;
- u32 *qp1_tunnel;
+ struct mlx4_spec_qps *spec_qps;
int num_srqs;
int max_srq_wqes;
int max_srq_sge;
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH RFC WIP 0/5] IGMP snooping for local traffic
From: Andrew Lunn @ 2017-08-28 13:45 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev, Vivien Didelot, nikolay, jiri, roopa, stephen, bridge
In-Reply-To: <b5bf300b-58c1-d7a7-8851-651683a234b0@gmail.com>
On Sun, Aug 27, 2017 at 07:44:15PM -0700, Florian Fainelli wrote:
> Hi Andrew,
>
> On 08/26/2017 01:56 PM, Andrew Lunn wrote:
> > This is a WIP patchset i would like comments on from bridge,
> > switchdev and hardware offload people.
> >
> > The linux bridge supports IGMP snooping. It will listen to IGMP
> > reports on bridge ports and keep track of which groups have been
> > joined on an interface. It will then forward multicast based on this
> > group membership.
> >
> > When the bridge adds or removed groups from an interface, it uses
> > switchdev to request the hardware add an mdb to a port, so the
> > hardware can perform the selective forwarding between ports.
> >
> > What is not covered by the current bridge code, is IGMP joins/leaves
> > from the host on the brX interface. No such monitoring is performed.
> > With a pure software bridge, it is not required. All mulitcast frames
> > are passed to the brX interface, and the network stack filters them,
> > as it does for any interface. However, when hardware offload is
> > involved, things change. We should program the hardware to only send
> > multcast packets to the host when the host has in interest in them.
>
> OK, so if I understand this right, without a bridge, we have the
> following happen today: with a DSA-enabled setup using any kind of
> switch tagging protocol, if a host is interested in receiving particular
> multicast traffic, we would receive IGMP joins/leaves through sw0p0, and
> the stack should call ndo_set_rx_mode for sw0p0, which would be
> dsa_slave_set_rx_mode() and which would synchronize the DSA master
> network device with the slave network device, everything works fine
> provided that the CPU port is configured to accept multicast traffic.
Hi Florian
That sounds about right, but it is not a use case i've looked at yet.
I do need to look at it though, because there is a chance i break it
with IGMP snooping support.
> Note here that we don't really add a MDB entry for sw0p0 when that
> happens, but it seems like we should for switches that lack IGMP
> snooping and/or multicast filtering.
Yes. But it is not an MDB in the normal sense, at least from the
switches perspective. An MDB passed to a switch using switchdev says
that traffic for the specified group should go out that port. However,
when the host does a join on sw0p0, we want the exact opposite,
multicast traffic coming in that port of the switch which matches the
group should be forwarded to the host. So my second attempt at this
code adds a new switchdev object, SWITCHDEV_OBJ_ID_PORT_HOST_MDB.
> With the current bridge and DSA code, are not we actually always going
> to get the CPU port to be added with the multicast address and therefore
> no filtering is occurring and snooping is pretty much useless?
Correct. At the moment, all multicast traffic gets delivered to the
host.
> While I understand the reasons why you did it that way, I think this is
> going to break a lot of code in bridge that does not expect brX to be a
> bridge port member.
It does.
Nikolay suggestion works a lot better, and i'm following that now. It
looks good and only requires some minor tweaks to the bridge code.
Andrew
^ permalink raw reply
* Re: Question about ip_defrag
From: Florian Westphal @ 2017-08-28 14:00 UTC (permalink / raw)
To: liujian (CE)
Cc: Jesper Dangaard Brouer, davem@davemloft.net, kuznet@ms2.inr.ac.ru,
yoshfuji@linux-ipv6.org, elena.reshetova@intel.com,
edumazet@google.com, netdev@vger.kernel.org, Wangkefeng (Kevin),
weiyongjun (A)
In-Reply-To: <4F88C5DDA1E80143B232E89585ACE27D018F3157@DGGEMA502-MBX.china.huawei.com>
liujian (CE) <liujian56@huawei.com> wrote:
> Hi
>
> I checked our 3.10 kernel, we had backported all percpu_counter bug fix in lib/percpu_counter.c and include/linux/percpu_counter.h.
> And I check 4.13-rc6, also has the issue if NIC's rx cpu num big enough.
>
> > > > > the issue:
> > > > > Ip_defrag fail caused by frag_mem_limit reached 4M(frags.high_thresh).
> > > > > At this moment,sum_frag_mem_limit is about 10K.
>
> So should we change ipfrag high/low thresh to a reasonable value ?
> And if it is, is there a standard to change the value?
Each cpu can have frag_percpu_counter_batch bytes rest doesn't know
about so with 64 cpus that is ~8 mbyte.
possible solutions:
1. reduce frag_percpu_counter_batch to 16k or so
2. make both low and high thresh depend on NR_CPUS
liujian, does this change help in any way?
diff --git a/net/ipv4/inet_fragment.c b/net/ipv4/inet_fragment.c
--- a/net/ipv4/inet_fragment.c
+++ b/net/ipv4/inet_fragment.c
@@ -123,6 +123,17 @@ static bool inet_fragq_should_evict(const struct inet_frag_queue *q)
frag_mem_limit(q->net) >= q->net->low_thresh;
}
+/* ->mem batch size is huge, this can cause severe discrepancies
+ * between actual value (sum of pcpu values) and the global estimate.
+ *
+ * Use a smaller batch to give an opportunity for the global estimate
+ * to more accurately reflect current state.
+ */
+static void update_frag_mem_limit(struct netns_frags *nf, unsigned int batch)
+{
+ percpu_counter_add_batch(&nf->mem, 0, batch);
+}
+
static unsigned int
inet_evict_bucket(struct inet_frags *f, struct inet_frag_bucket *hb)
{
@@ -146,8 +157,12 @@ inet_evict_bucket(struct inet_frags *f, struct inet_frag_bucket *hb)
spin_unlock(&hb->chain_lock);
- hlist_for_each_entry_safe(fq, n, &expired, list_evictor)
+ hlist_for_each_entry_safe(fq, n, &expired, list_evictor) {
+ struct netns_frags *nf = fq->net;
+
f->frag_expire((unsigned long) fq);
+ update_frag_mem_limit(nf, 1);
+ }
return evicted;
}
@@ -396,8 +411,10 @@ struct inet_frag_queue *inet_frag_find(struct netns_frags *nf,
struct inet_frag_queue *q;
int depth = 0;
- if (frag_mem_limit(nf) > nf->low_thresh)
+ if (frag_mem_limit(nf) > nf->low_thresh) {
inet_frag_schedule_worker(f);
+ update_frag_mem_limit(nf, SKB_TRUESIZE(1500) * 16);
+ }
hash &= (INETFRAGS_HASHSZ - 1);
hb = &f->hash[hash];
@@ -416,6 +433,8 @@ struct inet_frag_queue *inet_frag_find(struct netns_frags *nf,
if (depth <= INETFRAGS_MAXDEPTH)
return inet_frag_create(nf, f, key);
+ update_frag_mem_limit(nf, 1);
+
if (inet_frag_may_rebuild(f)) {
if (!f->rebuild)
f->rebuild = true;
^ permalink raw reply
* Re: [PATCH] connector: Delete an error message for a failed memory allocation in cn_queue_alloc_callback_entry()
From: Waskiewicz Jr, Peter @ 2017-08-28 14:05 UTC (permalink / raw)
To: Dan Carpenter
Cc: SF Markus Elfring, netdev@vger.kernel.org, Evgeniy Polyakov, LKML,
kernel-janitors@vger.kernel.org
In-Reply-To: <20170828060545.yddjpqzitw46lgiu@mwanda>
On 8/28/17 2:06 AM, Dan Carpenter wrote:
> On Sun, Aug 27, 2017 at 11:16:06PM +0000, Waskiewicz Jr, Peter wrote:
>> On 8/27/17 3:26 PM, SF Markus Elfring wrote:
>>> From: Markus Elfring <elfring@users.sourceforge.net>
>>> Date: Sun, 27 Aug 2017 21:18:37 +0200
>>>
>>> Omit an extra message for a memory allocation failure in this function.
>>>
>>> This issue was detected by using the Coccinelle software.
>>
>> Did coccinelle trip on the message or the fact you weren't returning NULL?
>>
>
> You've misread the patch somehow. The existing code has a NULL return
> and it's preserved in Markus's patch. This sort of patch is to fix a
> checkpatch.pl warning. The error message from this kzalloc() isn't going
> to get printed because it's a small allocation and small allocations
> always succeed in current kernels. But probably the main reason
> checkpatch complains is that kmalloc() already prints a stack trace and
> a bunch of other information so the printk doesn't add anyting.
> Removing it saves a little memory.
>
> I'm mostly a fan of running checkpatch on new patches or staging and not
> on old code...
And this is what I get for reading the patch with a crappy
mailer...thanks Doubtlook.
Sorry for the noise.
^ permalink raw reply
* Re: [PATCH] DSA support for Micrel KSZ8895
From: Andrew Lunn @ 2017-08-28 14:09 UTC (permalink / raw)
To: Pavel Machek
Cc: Woojung.Huh, nathan.leigh.conrad, vivien.didelot, f.fainelli,
netdev, linux-kernel, Tristram.Ha
In-Reply-To: <20170828070232.GA18135@amd>
> I may be confused here, but AFAICT:
>
> 1) Yes, it has standard layout when accessed over MDIO.
Section 4.8 of the datasheet says:
All the registers defined in this section can be also accessed
via the SPI interface.
Meaning all PHY registers can be access via the SPI interface. So you
should be able to make a standard Linux MDIO bus driver which performs
SPI reads.
Andrew
^ permalink raw reply
* [net-next PATCH 0/9] sockmap UAPI updates and fixes
From: John Fastabend @ 2017-08-28 14:09 UTC (permalink / raw)
To: ast, daniel, davem; +Cc: netdev, john.fastabend
This series updates sockmap UAPI, adds additional test cases and
provides a couple fixes.
First the UAPI changes. The original API added two sockmap specific
API artifacts (a) a new map_flags field with a sockmap specific update
command and (b) a new sockmap specific attach field in the attach data
structure. After this series instead of attaching programs with a
single command now two commands are used to attach programs to maps
individually. This allows us to add new programs easily in the future
and avoids any specific sockmap data structure additions. The
map_flags field is also removed and instead we allow socks to be
added to multiple maps that may or may not have programs attached.
This allows users to decide if a sock should run a SK_SKB program type
on receive based on the map it is attached to. This is a nice
improvement. See patches for specific details.
More test cases were added to test above changes and also stress test
the interface.
Finally two fixes/improvements were made. First a missing rcu
section was added. Second now sockmap can build without KCM being
used to trigger 'y' on CONFIG_STREAM_PARSER by selecting a new
BPF config option.
---
John Fastabend (9):
bpf: convert sockmap field attach_bpf_fd2 to type
bpf: sockmap, remove STRPARSER map_flags and add multi-map support
bpf: sockmap add missing rcu_read_(un)lock in smap_data_ready
bpf: additional sockmap self tests
bpf: more SK_SKB selftests
bpf: harden sockmap program attach to ensure correct map type
bpf: sockmap indicate sock events to listeners
bpf: sockmap requires STREAM_PARSER add Kconfig entry
bpf: test_maps add sockmap stress test
include/linux/bpf.h | 10 +
include/uapi/linux/bpf.h | 12 -
kernel/bpf/sockmap.c | 312 ++++++++++++--------
kernel/bpf/syscall.c | 38 +-
net/Kconfig | 12 +
samples/sockmap/sockmap_kern.c | 6
samples/sockmap/sockmap_user.c | 12 +
tools/include/uapi/linux/bpf.h | 9 -
tools/lib/bpf/bpf.c | 14 -
tools/lib/bpf/bpf.h | 4
tools/testing/selftests/bpf/bpf_helpers.h | 3
tools/testing/selftests/bpf/sockmap_parse_prog.c | 8 -
tools/testing/selftests/bpf/sockmap_verdict_prog.c | 30 ++
tools/testing/selftests/bpf/test_maps.c | 272 +++++++++++------
tools/testing/selftests/bpf/test_verifier.c | 98 ++++++
15 files changed, 544 insertions(+), 296 deletions(-)
^ permalink raw reply
* [net-next PATCH 1/9] bpf: convert sockmap field attach_bpf_fd2 to type
From: John Fastabend @ 2017-08-28 14:10 UTC (permalink / raw)
To: ast, daniel, davem; +Cc: netdev, john.fastabend
In-Reply-To: <20170828140850.14143.83953.stgit@john-Precision-Tower-5810>
In the initial sockmap API we provided strparser and verdict programs
using a single attach command by extending the attach API with a the
attach_bpf_fd2 field.
However, if we add other programs in the future we will be adding a
field for every new possible type, attach_bpf_fd(3,4,..). This
seems a bit clumsy for an API. So lets push the programs using two
new type fields.
BPF_SK_SKB_STREAM_PARSER
BPF_SK_SKB_STREAM_VERDICT
This has the advantage of having a readable name and can easily be
extended in the future.
Updates to samples and sockmap included here also generalize tests
slightly to support upcoming patch for multiple map support.
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
Fixes: 174a79ff9515 ("bpf: sockmap with sk redirect support")
Suggested-by: Alexei Starovoitov <ast@kernel.org>
---
include/linux/bpf.h | 10 +-
include/uapi/linux/bpf.h | 9 -
kernel/bpf/sockmap.c | 25 ++--
kernel/bpf/syscall.c | 38 ++----
samples/sockmap/sockmap_kern.c | 6 -
samples/sockmap/sockmap_user.c | 12 ++
tools/include/uapi/linux/bpf.h | 9 -
tools/lib/bpf/bpf.c | 14 --
tools/lib/bpf/bpf.h | 4 -
tools/testing/selftests/bpf/bpf_helpers.h | 3
tools/testing/selftests/bpf/sockmap_parse_prog.c | 2
tools/testing/selftests/bpf/sockmap_verdict_prog.c | 2
tools/testing/selftests/bpf/test_maps.c | 133 +++++++++-----------
13 files changed, 116 insertions(+), 151 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 830f472..c2cb1b5 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -39,8 +39,6 @@ struct bpf_map_ops {
void (*map_fd_put_ptr)(void *ptr);
u32 (*map_gen_lookup)(struct bpf_map *map, struct bpf_insn *insn_buf);
u32 (*map_fd_sys_lookup_elem)(void *ptr);
- int (*map_attach)(struct bpf_map *map,
- struct bpf_prog *p1, struct bpf_prog *p2);
};
struct bpf_map {
@@ -387,11 +385,19 @@ static inline void __dev_map_flush(struct bpf_map *map)
#if defined(CONFIG_STREAM_PARSER) && defined(CONFIG_BPF_SYSCALL)
struct sock *__sock_map_lookup_elem(struct bpf_map *map, u32 key);
+int sock_map_attach_prog(struct bpf_map *map, struct bpf_prog *prog, u32 type);
#else
static inline struct sock *__sock_map_lookup_elem(struct bpf_map *map, u32 key)
{
return NULL;
}
+
+static inline int sock_map_attach_prog(struct bpf_map *map,
+ struct bpf_prog *prog,
+ u32 type)
+{
+ return -EOPNOTSUPP;
+}
#endif
/* verifier prototypes for helper functions called from eBPF programs */
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 843818d..97227be 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -136,7 +136,8 @@ enum bpf_attach_type {
BPF_CGROUP_INET_EGRESS,
BPF_CGROUP_INET_SOCK_CREATE,
BPF_CGROUP_SOCK_OPS,
- BPF_CGROUP_SMAP_INGRESS,
+ BPF_SK_SKB_STREAM_PARSER,
+ BPF_SK_SKB_STREAM_VERDICT,
__MAX_BPF_ATTACH_TYPE
};
@@ -224,7 +225,6 @@ enum bpf_attach_type {
__u32 attach_bpf_fd; /* eBPF program to attach */
__u32 attach_type;
__u32 attach_flags;
- __u32 attach_bpf_fd2;
};
struct { /* anonymous struct used by BPF_PROG_TEST_RUN command */
@@ -580,14 +580,11 @@ enum bpf_attach_type {
* @flags: reserved for future use
* Return: SK_REDIRECT
*
- * int bpf_sock_map_update(skops, map, key, flags, map_flags)
+ * int bpf_sock_map_update(skops, map, key, flags)
* @skops: pointer to bpf_sock_ops
* @map: pointer to sockmap to update
* @key: key to insert/update sock in map
* @flags: same flags as map update elem
- * @map_flags: sock map specific flags
- * bit 1: Enable strparser
- * other bits: reserved
*/
#define __BPF_FUNC_MAPPER(FN) \
FN(unspec), \
diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c
index 617c239..cf570d1 100644
--- a/kernel/bpf/sockmap.c
+++ b/kernel/bpf/sockmap.c
@@ -723,20 +723,24 @@ static int sock_map_ctx_update_elem(struct bpf_sock_ops_kern *skops,
return err;
}
-static int sock_map_attach_prog(struct bpf_map *map,
- struct bpf_prog *parse,
- struct bpf_prog *verdict)
+int sock_map_attach_prog(struct bpf_map *map, struct bpf_prog *prog, u32 type)
{
struct bpf_stab *stab = container_of(map, struct bpf_stab, map);
- struct bpf_prog *_parse, *_verdict;
+ struct bpf_prog *orig;
- _parse = xchg(&stab->bpf_parse, parse);
- _verdict = xchg(&stab->bpf_verdict, verdict);
+ switch (type) {
+ case BPF_SK_SKB_STREAM_PARSER:
+ orig = xchg(&stab->bpf_parse, prog);
+ break;
+ case BPF_SK_SKB_STREAM_VERDICT:
+ orig = xchg(&stab->bpf_verdict, prog);
+ break;
+ default:
+ return -EOPNOTSUPP;
+ }
- if (_parse)
- bpf_prog_put(_parse);
- if (_verdict)
- bpf_prog_put(_verdict);
+ if (orig)
+ bpf_prog_put(orig);
return 0;
}
@@ -777,7 +781,6 @@ static int sock_map_update_elem(struct bpf_map *map,
.map_get_next_key = sock_map_get_next_key,
.map_update_elem = sock_map_update_elem,
.map_delete_elem = sock_map_delete_elem,
- .map_attach = sock_map_attach_prog,
};
BPF_CALL_5(bpf_sock_map_update, struct bpf_sock_ops_kern *, bpf_sock,
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 9378f3b..021a05d 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1093,12 +1093,12 @@ static int bpf_obj_get(const union bpf_attr *attr)
#ifdef CONFIG_CGROUP_BPF
-#define BPF_PROG_ATTACH_LAST_FIELD attach_bpf_fd2
+#define BPF_PROG_ATTACH_LAST_FIELD attach_flags
-static int sockmap_get_from_fd(const union bpf_attr *attr, int ptype)
+static int sockmap_get_from_fd(const union bpf_attr *attr)
{
- struct bpf_prog *prog1, *prog2;
int ufd = attr->target_fd;
+ struct bpf_prog *prog;
struct bpf_map *map;
struct fd f;
int err;
@@ -1108,29 +1108,16 @@ static int sockmap_get_from_fd(const union bpf_attr *attr, int ptype)
if (IS_ERR(map))
return PTR_ERR(map);
- if (!map->ops->map_attach) {
- fdput(f);
- return -EOPNOTSUPP;
- }
-
- prog1 = bpf_prog_get_type(attr->attach_bpf_fd, ptype);
- if (IS_ERR(prog1)) {
+ prog = bpf_prog_get_type(attr->attach_bpf_fd, BPF_PROG_TYPE_SK_SKB);
+ if (IS_ERR(prog)) {
fdput(f);
- return PTR_ERR(prog1);
- }
-
- prog2 = bpf_prog_get_type(attr->attach_bpf_fd2, ptype);
- if (IS_ERR(prog2)) {
- fdput(f);
- bpf_prog_put(prog1);
- return PTR_ERR(prog2);
+ return PTR_ERR(prog);
}
- err = map->ops->map_attach(map, prog1, prog2);
+ err = sock_map_attach_prog(map, prog, attr->attach_type);
if (err) {
fdput(f);
- bpf_prog_put(prog1);
- bpf_prog_put(prog2);
+ bpf_prog_put(prog);
return err;
}
@@ -1165,16 +1152,13 @@ static int bpf_prog_attach(const union bpf_attr *attr)
case BPF_CGROUP_SOCK_OPS:
ptype = BPF_PROG_TYPE_SOCK_OPS;
break;
- case BPF_CGROUP_SMAP_INGRESS:
- ptype = BPF_PROG_TYPE_SK_SKB;
- break;
+ case BPF_SK_SKB_STREAM_PARSER:
+ case BPF_SK_SKB_STREAM_VERDICT:
+ return sockmap_get_from_fd(attr);
default:
return -EINVAL;
}
- if (attr->attach_type == BPF_CGROUP_SMAP_INGRESS)
- return sockmap_get_from_fd(attr, ptype);
-
prog = bpf_prog_get_type(attr->attach_bpf_fd, ptype);
if (IS_ERR(prog))
return PTR_ERR(prog);
diff --git a/samples/sockmap/sockmap_kern.c b/samples/sockmap/sockmap_kern.c
index 6ff986f..f9b38ef 100644
--- a/samples/sockmap/sockmap_kern.c
+++ b/samples/sockmap/sockmap_kern.c
@@ -82,8 +82,7 @@ int bpf_sockmap(struct bpf_sock_ops *skops)
if (lport == 10000) {
ret = 1;
err = bpf_sock_map_update(skops, &sock_map, &ret,
- BPF_NOEXIST,
- BPF_SOCKMAP_STRPARSER);
+ BPF_NOEXIST);
bpf_printk("passive(%i -> %i) map ctx update err: %d\n",
lport, bpf_ntohl(rport), err);
}
@@ -95,8 +94,7 @@ int bpf_sockmap(struct bpf_sock_ops *skops)
if (bpf_ntohl(rport) == 10001) {
ret = 10;
err = bpf_sock_map_update(skops, &sock_map, &ret,
- BPF_NOEXIST,
- BPF_SOCKMAP_STRPARSER);
+ BPF_NOEXIST);
bpf_printk("active(%i -> %i) map ctx update err: %d\n",
lport, bpf_ntohl(rport), err);
}
diff --git a/samples/sockmap/sockmap_user.c b/samples/sockmap/sockmap_user.c
index fb78f5a..7cc9d22 100644
--- a/samples/sockmap/sockmap_user.c
+++ b/samples/sockmap/sockmap_user.c
@@ -256,8 +256,16 @@ int main(int argc, char **argv)
}
/* Attach programs to sockmap */
- err = __bpf_prog_attach(prog_fd[0], prog_fd[1], map_fd[0],
- BPF_CGROUP_SMAP_INGRESS, 0);
+ err = bpf_prog_attach(prog_fd[0], map_fd[0],
+ BPF_SK_SKB_STREAM_PARSER, 0);
+ if (err) {
+ fprintf(stderr, "ERROR: bpf_prog_attach (sockmap): %d (%s)\n",
+ err, strerror(errno));
+ return err;
+ }
+
+ err = bpf_prog_attach(prog_fd[1], map_fd[0],
+ BPF_SK_SKB_STREAM_VERDICT, 0);
if (err) {
fprintf(stderr, "ERROR: bpf_prog_attach (sockmap): %d (%s)\n",
err, strerror(errno));
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index f8f6377..09ac590 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -136,7 +136,8 @@ enum bpf_attach_type {
BPF_CGROUP_INET_EGRESS,
BPF_CGROUP_INET_SOCK_CREATE,
BPF_CGROUP_SOCK_OPS,
- BPF_CGROUP_SMAP_INGRESS,
+ BPF_SK_SKB_STREAM_PARSER,
+ BPF_SK_SKB_STREAM_VERDICT,
__MAX_BPF_ATTACH_TYPE
};
@@ -227,7 +228,6 @@ enum bpf_sockmap_flags {
__u32 attach_bpf_fd; /* eBPF program to attach */
__u32 attach_type;
__u32 attach_flags;
- __u32 attach_bpf_fd2;
};
struct { /* anonymous struct used by BPF_PROG_TEST_RUN command */
@@ -572,14 +572,11 @@ enum bpf_sockmap_flags {
* @flags: reserved for future use
* Return: SK_REDIRECT
*
- * int bpf_sock_map_update(skops, map, key, flags, map_flags)
+ * int bpf_sock_map_update(skops, map, key, flags)
* @skops: pointer to bpf_sock_ops
* @map: pointer to sockmap to update
* @key: key to insert/update sock in map
* @flags: same flags as map update elem
- * @map_flags: sock map specific flags
- * bit 1: Enable strparser
- * other bits: reserved
*/
#define __BPF_FUNC_MAPPER(FN) \
FN(unspec), \
diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
index a071761..1d6907d 100644
--- a/tools/lib/bpf/bpf.c
+++ b/tools/lib/bpf/bpf.c
@@ -235,28 +235,20 @@ int bpf_obj_get(const char *pathname)
return sys_bpf(BPF_OBJ_GET, &attr, sizeof(attr));
}
-int __bpf_prog_attach(int prog_fd1, int prog_fd2, int target_fd,
- enum bpf_attach_type type,
- unsigned int flags)
+int bpf_prog_attach(int prog_fd, int target_fd, enum bpf_attach_type type,
+ unsigned int flags)
{
union bpf_attr attr;
bzero(&attr, sizeof(attr));
attr.target_fd = target_fd;
- attr.attach_bpf_fd = prog_fd1;
- attr.attach_bpf_fd2 = prog_fd2;
+ attr.attach_bpf_fd = prog_fd;
attr.attach_type = type;
attr.attach_flags = flags;
return sys_bpf(BPF_PROG_ATTACH, &attr, sizeof(attr));
}
-int bpf_prog_attach(int prog_fd, int target_fd, enum bpf_attach_type type,
- unsigned int flags)
-{
- return __bpf_prog_attach(prog_fd, 0, target_fd, type, flags);
-}
-
int bpf_prog_detach(int target_fd, enum bpf_attach_type type)
{
union bpf_attr attr;
diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
index 90e9d4e..b8ea584 100644
--- a/tools/lib/bpf/bpf.h
+++ b/tools/lib/bpf/bpf.h
@@ -56,10 +56,6 @@ int bpf_map_update_elem(int fd, const void *key, const void *value,
int bpf_obj_get(const char *pathname);
int bpf_prog_attach(int prog_fd, int attachable_fd, enum bpf_attach_type type,
unsigned int flags);
-int __bpf_prog_attach(int prog1, int prog2,
- int attachable_fd,
- enum bpf_attach_type type,
- unsigned int flags);
int bpf_prog_detach(int attachable_fd, enum bpf_attach_type type);
int bpf_prog_test_run(int prog_fd, int repeat, void *data, __u32 size,
void *data_out, __u32 *size_out, __u32 *retval,
diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
index 98f3be2..36fb916 100644
--- a/tools/testing/selftests/bpf/bpf_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_helpers.h
@@ -68,8 +68,7 @@ static int (*bpf_setsockopt)(void *ctx, int level, int optname, void *optval,
static int (*bpf_sk_redirect_map)(void *map, int key, int flags) =
(void *) BPF_FUNC_sk_redirect_map;
static int (*bpf_sock_map_update)(void *map, void *key, void *value,
- unsigned long long flags,
- unsigned long long map_lags) =
+ unsigned long long flags) =
(void *) BPF_FUNC_sock_map_update;
diff --git a/tools/testing/selftests/bpf/sockmap_parse_prog.c b/tools/testing/selftests/bpf/sockmap_parse_prog.c
index 8b54531..710f43f 100644
--- a/tools/testing/selftests/bpf/sockmap_parse_prog.c
+++ b/tools/testing/selftests/bpf/sockmap_parse_prog.c
@@ -30,7 +30,7 @@ int bpf_prog1(struct __sk_buff *skb)
*/
d[0] = 1;
- bpf_printk("data[0] = (%u): local_port %i remote %i\n",
+ bpf_printk("parse: data[0] = (%u): local_port %i remote %i\n",
d[0], lport, bpf_ntohl(rport));
return skb->len;
}
diff --git a/tools/testing/selftests/bpf/sockmap_verdict_prog.c b/tools/testing/selftests/bpf/sockmap_verdict_prog.c
index d5f9447..0573c1d 100644
--- a/tools/testing/selftests/bpf/sockmap_verdict_prog.c
+++ b/tools/testing/selftests/bpf/sockmap_verdict_prog.c
@@ -40,7 +40,7 @@ int bpf_prog2(struct __sk_buff *skb)
d[6] = 0xe;
d[7] = 0xf;
- bpf_printk("data[0] = (%u): local_port %i remote %i\n",
+ bpf_printk("verdict: data[0] = (%u): local_port %i remote %i redirect 5\n",
d[0], lport, bpf_ntohl(rport));
return bpf_sk_redirect_map(&sock_map, 5, 0);
}
diff --git a/tools/testing/selftests/bpf/test_maps.c b/tools/testing/selftests/bpf/test_maps.c
index 40b2d1f..6df6e62 100644
--- a/tools/testing/selftests/bpf/test_maps.c
+++ b/tools/testing/selftests/bpf/test_maps.c
@@ -547,20 +547,26 @@ static void test_sockmap(int task, void *data)
goto out_sockmap;
}
- /* Nothing attached so these should fail */
+ /* Test update without programs */
for (i = 0; i < 6; i++) {
err = bpf_map_update_elem(fd, &i, &sfd[i], BPF_ANY);
- if (!err) {
- printf("Failed invalid update sockmap '%i:%i'\n",
+ if (err) {
+ printf("Failed noprog update sockmap '%i:%i'\n",
i, sfd[i]);
goto out_sockmap;
}
}
/* Test attaching bad fds */
- err = __bpf_prog_attach(-1, -2, fd, BPF_CGROUP_SMAP_INGRESS, 0);
+ err = bpf_prog_attach(-1, fd, BPF_SK_SKB_STREAM_PARSER, 0);
if (!err) {
- printf("Failed invalid prog attach\n");
+ printf("Failed invalid parser prog attach\n");
+ goto out_sockmap;
+ }
+
+ err = bpf_prog_attach(-1, fd, BPF_SK_SKB_STREAM_VERDICT, 0);
+ if (!err) {
+ printf("Failed invalid verdict prog attach\n");
goto out_sockmap;
}
@@ -591,14 +597,21 @@ static void test_sockmap(int task, void *data)
goto out_sockmap;
}
- err = __bpf_prog_attach(parse_prog, verdict_prog, map_fd,
- BPF_CGROUP_SMAP_INGRESS, 0);
+ err = bpf_prog_attach(parse_prog, map_fd,
+ BPF_SK_SKB_STREAM_PARSER, 0);
+ if (err) {
+ printf("Failed bpf prog attach\n");
+ goto out_sockmap;
+ }
+
+ err = bpf_prog_attach(verdict_prog, map_fd,
+ BPF_SK_SKB_STREAM_VERDICT, 0);
if (err) {
printf("Failed bpf prog attach\n");
goto out_sockmap;
}
- /* Test map update elem */
+ /* Test map update elem afterwards fd lives in fd and map_fd */
for (i = 0; i < 6; i++) {
err = bpf_map_update_elem(map_fd, &i, &sfd[i], BPF_ANY);
if (err) {
@@ -649,96 +662,68 @@ static void test_sockmap(int task, void *data)
goto out_sockmap;
}
- /* Delete the reset of the elems include some NULL elems */
- for (i = 0; i < 6; i++) {
- err = bpf_map_delete_elem(map_fd, &i);
- if (err && (i == 0 || i == 1 || i >= 4)) {
- printf("Failed delete sockmap %i '%i:%i'\n",
- err, i, sfd[i]);
- goto out_sockmap;
- } else if (!err && (i == 2 || i == 3)) {
- printf("Failed null delete sockmap %i '%i:%i'\n",
- err, i, sfd[i]);
- goto out_sockmap;
- }
- }
-
- /* Test having multiple SMAPs open and active on same fds */
- err = __bpf_prog_attach(parse_prog, verdict_prog, fd,
- BPF_CGROUP_SMAP_INGRESS, 0);
- if (err) {
- printf("Failed fd bpf prog attach\n");
- goto out_sockmap;
- }
-
- for (i = 0; i < 6; i++) {
- err = bpf_map_update_elem(fd, &i, &sfd[i], BPF_ANY);
- if (err) {
- printf("Failed fd update sockmap %i '%i:%i'\n",
- err, i, sfd[i]);
- goto out_sockmap;
- }
- }
-
- /* Test duplicate socket add of NOEXIST, ANY and EXIST */
- i = 0;
+ /* Push fd into same slot */
+ i = 2;
err = bpf_map_update_elem(fd, &i, &sfd[i], BPF_NOEXIST);
if (!err) {
- printf("Failed BPF_NOEXIST create\n");
+ printf("Failed allowed sockmap dup slot BPF_NOEXIST\n");
goto out_sockmap;
}
err = bpf_map_update_elem(fd, &i, &sfd[i], BPF_ANY);
if (err) {
- printf("Failed sockmap update BPF_ANY\n");
+ printf("Failed sockmap update new slot BPF_ANY\n");
goto out_sockmap;
}
err = bpf_map_update_elem(fd, &i, &sfd[i], BPF_EXIST);
if (err) {
- printf("Failed sockmap update BPF_EXIST\n");
+ printf("Failed sockmap update new slot BPF_EXIST\n");
goto out_sockmap;
}
- /* The above were pushing fd into same slot try different slot now */
- i = 2;
- err = bpf_map_update_elem(fd, &i, &sfd[i], BPF_NOEXIST);
- if (!err) {
- printf("Failed BPF_NOEXIST create\n");
- goto out_sockmap;
+ /* Delete the elems without programs */
+ for (i = 0; i < 6; i++) {
+ err = bpf_map_delete_elem(fd, &i);
+ if (err) {
+ printf("Failed delete sockmap %i '%i:%i'\n",
+ err, i, sfd[i]);
+ }
}
- err = bpf_map_update_elem(fd, &i, &sfd[i], BPF_ANY);
+ /* Test having multiple maps open and set with programs on same fds */
+ err = bpf_prog_attach(parse_prog, fd,
+ BPF_SK_SKB_STREAM_PARSER, 0);
if (err) {
- printf("Failed sockmap update BPF_ANY\n");
+ printf("Failed fd bpf parse prog attach\n");
goto out_sockmap;
}
-
- err = bpf_map_update_elem(fd, &i, &sfd[i], BPF_EXIST);
+ err = bpf_prog_attach(verdict_prog, fd,
+ BPF_SK_SKB_STREAM_VERDICT, 0);
if (err) {
- printf("Failed sockmap update BPF_EXIST\n");
+ printf("Failed fd bpf verdict prog attach\n");
goto out_sockmap;
}
- /* Try pushing fd into different map, this is not allowed at the
- * moment. Which programs would we use?
- */
- err = bpf_map_update_elem(map_fd, &i, &sfd[i], BPF_NOEXIST);
- if (!err) {
- printf("Failed BPF_NOEXIST create\n");
- goto out_sockmap;
- }
-
- err = bpf_map_update_elem(map_fd, &i, &sfd[i], BPF_ANY);
- if (!err) {
- printf("Failed sockmap update BPF_ANY\n");
- goto out_sockmap;
- }
-
- err = bpf_map_update_elem(map_fd, &i, &sfd[i], BPF_EXIST);
- if (!err) {
- printf("Failed sockmap update BPF_EXIST\n");
- goto out_sockmap;
+ for (i = 4; i < 6; i++) {
+ err = bpf_map_update_elem(fd, &i, &sfd[i], BPF_ANY);
+ if (!err) {
+ printf("Failed allowed duplicate programs in update ANY sockmap %i '%i:%i'\n",
+ err, i, sfd[i]);
+ goto out_sockmap;
+ }
+ err = bpf_map_update_elem(fd, &i, &sfd[i], BPF_NOEXIST);
+ if (!err) {
+ printf("Failed allowed duplicate program in update NOEXIST sockmap %i '%i:%i'\n",
+ err, i, sfd[i]);
+ goto out_sockmap;
+ }
+ err = bpf_map_update_elem(fd, &i, &sfd[i], BPF_EXIST);
+ if (!err) {
+ printf("Failed allowed duplicate program in update EXIST sockmap %i '%i:%i'\n",
+ err, i, sfd[i]);
+ goto out_sockmap;
+ }
}
/* Test map close sockets */
^ permalink raw reply related
* [net-next PATCH 2/9] bpf: sockmap, remove STRPARSER map_flags and add multi-map support
From: John Fastabend @ 2017-08-28 14:10 UTC (permalink / raw)
To: ast, daniel, davem; +Cc: netdev, john.fastabend
In-Reply-To: <20170828140850.14143.83953.stgit@john-Precision-Tower-5810>
The addition of map_flags BPF_SOCKMAP_STRPARSER flags was to handle a
specific use case where we want to have BPF parse program disabled on
an entry in a sockmap.
However, Alexei found the API a bit cumbersome and I agreed. Lets
remove the STRPARSER flag and support the use case by allowing socks
to be in multiple maps. This allows users to create two maps one with
programs attached and one without. When socks are added to maps they
now inherit any programs attached to the map. This is a nice
generalization and IMO improves the API.
The API rules are less ambiguous and do not need a flag:
- When a sock is added to a sockmap we have two cases,
i. The sock map does not have any attached programs so
we can add sock to map without inheriting bpf programs.
The sock may exist in 0 or more other maps.
ii. The sock map has an attached BPF program. To avoid duplicate
bpf programs we only add the sock entry if it does not have
an existing strparser/verdict attached, returning -EBUSY if
a program is already attached. Otherwise attach the program
and inherit strparser/verdict programs from the sock map.
This allows for socks to be in a multiple maps for redirects and
inherit a BPF program from a single map.
Also this patch simplifies the logic around BPF_{EXIST|NOEXIST|ANY}
flags. In the original patch I tried to be extra clever and only
update map entries when necessary. Now I've decided the complexity
is not worth it. If users constantly update an entry with the same
sock for no reason (i.e. update an entry without actually changing
any parameters on map or sock) we still do an alloc/release. Using
this and allowing multiple entries of a sock to exist in a map the
logic becomes much simpler.
Note: Now that multiple maps are supported the "maps" pointer called
when a socket is closed becomes a list of maps to remove the sock from.
To keep the map up to date when a sock is added to the sockmap we must
add the map/elem in the list. Likewise when it is removed we must
remove it from the list. This results in searching the per psock list
on delete operation. On TCP_CLOSE events we walk the list and remove
the psock from all map/entry locations. I don't see any perf
implications in this because at most I have a psock in two maps. If
a psock were to be in many maps its possibly this might be noticeable
on delete but I can't think of a reason to dup a psock in many maps.
The sk_callback_lock is used to protect read/writes to the list. This
was convenient because in all locations we were taking the lock
anyways just after working on the list. Also the lock is per sock so
in normal cases we shouldn't see any contention.
Suggested-by: Alexei Starovoitov <ast@kernel.org>
Fixes: 174a79ff9515 ("bpf: sockmap with sk redirect support")
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
include/uapi/linux/bpf.h | 3 -
kernel/bpf/sockmap.c | 269 ++++++++++++++++++++++++++++------------------
2 files changed, 165 insertions(+), 107 deletions(-)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 97227be..08c206a 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -143,9 +143,6 @@ enum bpf_attach_type {
#define MAX_BPF_ATTACH_TYPE __MAX_BPF_ATTACH_TYPE
-/* If BPF_SOCKMAP_STRPARSER is used sockmap will use strparser on receive */
-#define BPF_SOCKMAP_STRPARSER (1U << 0)
-
/* If BPF_F_ALLOW_OVERRIDE flag is used in BPF_PROG_ATTACH command
* to the given target_fd cgroup the descendent cgroup will be able to
* override effective bpf program that was inherited from this cgroup
diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c
index cf570d1..a6882e5 100644
--- a/kernel/bpf/sockmap.c
+++ b/kernel/bpf/sockmap.c
@@ -13,15 +13,16 @@
/* A BPF sock_map is used to store sock objects. This is primarly used
* for doing socket redirect with BPF helper routines.
*
- * A sock map may have two BPF programs attached to it, a program used
- * to parse packets and a program to provide a verdict and redirect
- * decision on the packet. If no BPF parse program is provided it is
- * assumed that every skb is a "message" (skb->len). Otherwise the
- * parse program is attached to strparser and used to build messages
- * that may span multiple skbs. The verdict program will either select
- * a socket to send/receive the skb on or provide the drop code indicating
- * the skb should be dropped. More actions may be added later as needed.
- * The default program will drop packets.
+ * A sock map may have BPF programs attached to it, currently a program
+ * used to parse packets and a program to provide a verdict and redirect
+ * decision on the packet are supported. Any programs attached to a sock
+ * map are inherited by sock objects when they are added to the map. If
+ * no BPF programs are attached the sock object may only be used for sock
+ * redirect.
+ *
+ * A sock object may be in multiple maps, but can only inherit a single
+ * parse or verdict program. If adding a sock object to a map would result
+ * in having multiple parsing programs the update will return an EBUSY error.
*
* For reference this program is similar to devmap used in XDP context
* reviewing these together may be useful. For an example please review
@@ -44,15 +45,21 @@ struct bpf_stab {
struct sock **sock_map;
struct bpf_prog *bpf_parse;
struct bpf_prog *bpf_verdict;
- refcount_t refcnt;
};
enum smap_psock_state {
SMAP_TX_RUNNING,
};
+struct smap_psock_map_entry {
+ struct list_head list;
+ struct sock **entry;
+};
+
struct smap_psock {
struct rcu_head rcu;
+ /* refcnt is used inside sk_callback_lock */
+ u32 refcnt;
/* datapath variables */
struct sk_buff_head rxqueue;
@@ -66,10 +73,9 @@ struct smap_psock {
struct strparser strp;
struct bpf_prog *bpf_parse;
struct bpf_prog *bpf_verdict;
- struct bpf_stab *stab;
+ struct list_head maps;
/* Back reference used when sock callback trigger sockmap operations */
- int key;
struct sock *sock;
unsigned long state;
@@ -83,7 +89,7 @@ struct smap_psock {
static inline struct smap_psock *smap_psock_sk(const struct sock *sk)
{
- return (struct smap_psock *)rcu_dereference_sk_user_data(sk);
+ return rcu_dereference_sk_user_data(sk);
}
static int smap_verdict_func(struct smap_psock *psock, struct sk_buff *skb)
@@ -149,11 +155,12 @@ static void smap_report_sk_error(struct smap_psock *psock, int err)
sk->sk_error_report(sk);
}
-static void smap_release_sock(struct sock *sock);
+static void smap_release_sock(struct smap_psock *psock, struct sock *sock);
/* Called with lock_sock(sk) held */
static void smap_state_change(struct sock *sk)
{
+ struct smap_psock_map_entry *e, *tmp;
struct smap_psock *psock;
struct sock *osk;
@@ -184,9 +191,15 @@ static void smap_state_change(struct sock *sk)
psock = smap_psock_sk(sk);
if (unlikely(!psock))
break;
- osk = cmpxchg(&psock->stab->sock_map[psock->key], sk, NULL);
- if (osk == sk)
- smap_release_sock(sk);
+ write_lock_bh(&sk->sk_callback_lock);
+ list_for_each_entry_safe(e, tmp, &psock->maps, list) {
+ osk = cmpxchg(e->entry, sk, NULL);
+ if (osk == sk) {
+ list_del(&e->list);
+ smap_release_sock(psock, sk);
+ }
+ }
+ write_unlock_bh(&sk->sk_callback_lock);
break;
default:
psock = smap_psock_sk(sk);
@@ -289,9 +302,8 @@ static void smap_write_space(struct sock *sk)
static void smap_stop_sock(struct smap_psock *psock, struct sock *sk)
{
- write_lock_bh(&sk->sk_callback_lock);
if (!psock->strp_enabled)
- goto out;
+ return;
sk->sk_data_ready = psock->save_data_ready;
sk->sk_write_space = psock->save_write_space;
sk->sk_state_change = psock->save_state_change;
@@ -300,8 +312,6 @@ static void smap_stop_sock(struct smap_psock *psock, struct sock *sk)
psock->save_state_change = NULL;
strp_stop(&psock->strp);
psock->strp_enabled = false;
-out:
- write_unlock_bh(&sk->sk_callback_lock);
}
static void smap_destroy_psock(struct rcu_head *rcu)
@@ -318,9 +328,11 @@ static void smap_destroy_psock(struct rcu_head *rcu)
schedule_work(&psock->gc_work);
}
-static void smap_release_sock(struct sock *sock)
+static void smap_release_sock(struct smap_psock *psock, struct sock *sock)
{
- struct smap_psock *psock = smap_psock_sk(sock);
+ psock->refcnt--;
+ if (psock->refcnt)
+ return;
smap_stop_sock(psock, sock);
clear_bit(SMAP_TX_RUNNING, &psock->state);
@@ -414,6 +426,7 @@ static void sock_map_remove_complete(struct bpf_stab *stab)
static void smap_gc_work(struct work_struct *w)
{
+ struct smap_psock_map_entry *e, *tmp;
struct smap_psock *psock;
psock = container_of(w, struct smap_psock, gc_work);
@@ -431,8 +444,10 @@ static void smap_gc_work(struct work_struct *w)
if (psock->bpf_verdict)
bpf_prog_put(psock->bpf_verdict);
- if (refcount_dec_and_test(&psock->stab->refcnt))
- sock_map_remove_complete(psock->stab);
+ list_for_each_entry_safe(e, tmp, &psock->maps, list) {
+ list_del(&e->list);
+ kfree(e);
+ }
sock_put(psock->sock);
kfree(psock);
@@ -453,6 +468,8 @@ static struct smap_psock *smap_init_psock(struct sock *sock,
skb_queue_head_init(&psock->rxqueue);
INIT_WORK(&psock->tx_work, smap_tx_work);
INIT_WORK(&psock->gc_work, smap_gc_work);
+ INIT_LIST_HEAD(&psock->maps);
+ psock->refcnt = 1;
rcu_assign_sk_user_data(sock, psock);
sock_hold(sock);
@@ -503,13 +520,24 @@ static struct bpf_map *sock_map_alloc(union bpf_attr *attr)
if (!stab->sock_map)
goto free_stab;
- refcount_set(&stab->refcnt, 1);
return &stab->map;
free_stab:
kfree(stab);
return ERR_PTR(err);
}
+static void smap_list_remove(struct smap_psock *psock, struct sock **entry)
+{
+ struct smap_psock_map_entry *e, *tmp;
+
+ list_for_each_entry_safe(e, tmp, &psock->maps, list) {
+ if (e->entry == entry) {
+ list_del(&e->list);
+ break;
+ }
+ }
+}
+
static void sock_map_free(struct bpf_map *map)
{
struct bpf_stab *stab = container_of(map, struct bpf_stab, map);
@@ -526,13 +554,18 @@ static void sock_map_free(struct bpf_map *map)
*/
rcu_read_lock();
for (i = 0; i < stab->map.max_entries; i++) {
+ struct smap_psock *psock;
struct sock *sock;
sock = xchg(&stab->sock_map[i], NULL);
if (!sock)
continue;
- smap_release_sock(sock);
+ write_lock_bh(&sock->sk_callback_lock);
+ psock = smap_psock_sk(sock);
+ smap_list_remove(psock, &stab->sock_map[i]);
+ smap_release_sock(psock, sock);
+ write_unlock_bh(&sock->sk_callback_lock);
}
rcu_read_unlock();
@@ -541,8 +574,7 @@ static void sock_map_free(struct bpf_map *map)
if (stab->bpf_parse)
bpf_prog_put(stab->bpf_parse);
- if (refcount_dec_and_test(&stab->refcnt))
- sock_map_remove_complete(stab);
+ sock_map_remove_complete(stab);
}
static int sock_map_get_next_key(struct bpf_map *map, void *key, void *next_key)
@@ -576,6 +608,7 @@ struct sock *__sock_map_lookup_elem(struct bpf_map *map, u32 key)
static int sock_map_delete_elem(struct bpf_map *map, void *key)
{
struct bpf_stab *stab = container_of(map, struct bpf_stab, map);
+ struct smap_psock *psock;
int k = *(u32 *)key;
struct sock *sock;
@@ -586,7 +619,17 @@ static int sock_map_delete_elem(struct bpf_map *map, void *key)
if (!sock)
return -EINVAL;
- smap_release_sock(sock);
+ write_lock_bh(&sock->sk_callback_lock);
+ psock = smap_psock_sk(sock);
+ if (!psock)
+ goto out;
+
+ if (psock->bpf_parse)
+ smap_stop_sock(psock, sock);
+ smap_list_remove(psock, &stab->sock_map[k]);
+ smap_release_sock(psock, sock);
+out:
+ write_unlock_bh(&sock->sk_callback_lock);
return 0;
}
@@ -601,29 +644,34 @@ static int sock_map_delete_elem(struct bpf_map *map, void *key)
* and syncd so we are certain all references from the update/lookup/delete
* operations as well as references in the data path are no longer in use.
*
- * A psock object holds a refcnt on the sockmap it is attached to and this is
- * not decremented until after a RCU grace period and garbage collection occurs.
- * This ensures the map is not free'd until psocks linked to it are removed. The
- * map link is used when the independent sock events trigger map deletion.
+ * Psocks may exist in multiple maps, but only a single set of parse/verdict
+ * programs may be inherited from the maps it belongs to. A reference count
+ * is kept with the total number of references to the psock from all maps. The
+ * psock will not be released until this reaches zero. The psock and sock
+ * user data data use the sk_callback_lock to protect critical data structures
+ * from concurrent access. This allows us to avoid two updates from modifying
+ * the user data in sock and the lock is required anyways for modifying
+ * callbacks, we simply increase its scope slightly.
*
- * Psocks may only participate in one sockmap at a time. Users that try to
- * join a single sock to multiple maps will get an error.
- *
- * Last, but not least, it is possible the socket is closed while running
- * an update on an existing psock. This will release the psock, but again
- * not until the update has completed due to rcu grace period rules.
+ * Rules to follow,
+ * - psock must always be read inside RCU critical section
+ * - sk_user_data must only be modified inside sk_callback_lock and read
+ * inside RCU critical section.
+ * - psock->maps list must only be read & modified inside sk_callback_lock
+ * - sock_map must use READ_ONCE and (cmp)xchg operations
+ * - BPF verdict/parse programs must use READ_ONCE and xchg operations
*/
static int sock_map_ctx_update_elem(struct bpf_sock_ops_kern *skops,
struct bpf_map *map,
- void *key, u64 flags, u64 map_flags)
+ void *key, u64 flags)
{
struct bpf_stab *stab = container_of(map, struct bpf_stab, map);
+ struct smap_psock_map_entry *e = NULL;
struct bpf_prog *verdict, *parse;
- struct smap_psock *psock = NULL;
- struct sock *old_sock, *sock;
+ struct sock *osock, *sock;
+ struct smap_psock *psock;
u32 i = *(u32 *)key;
- bool update = false;
- int err = 0;
+ int err;
if (unlikely(flags > BPF_EXIST))
return -EINVAL;
@@ -631,35 +679,22 @@ static int sock_map_ctx_update_elem(struct bpf_sock_ops_kern *skops,
if (unlikely(i >= stab->map.max_entries))
return -E2BIG;
- if (unlikely(map_flags > BPF_SOCKMAP_STRPARSER))
- return -EINVAL;
-
- verdict = parse = NULL;
sock = READ_ONCE(stab->sock_map[i]);
-
- if (flags == BPF_EXIST || flags == BPF_ANY) {
- if (!sock && flags == BPF_EXIST) {
- return -ENOENT;
- } else if (sock && sock != skops->sk) {
- return -EINVAL;
- } else if (sock) {
- psock = smap_psock_sk(sock);
- if (unlikely(!psock))
- return -EBUSY;
- update = true;
- }
- } else if (sock && BPF_NOEXIST) {
+ if (flags == BPF_EXIST && !sock)
+ return -ENOENT;
+ else if (flags == BPF_NOEXIST && sock)
return -EEXIST;
- }
- /* reserve BPF programs early so can abort easily on failures */
- if (map_flags & BPF_SOCKMAP_STRPARSER) {
- verdict = READ_ONCE(stab->bpf_verdict);
- parse = READ_ONCE(stab->bpf_parse);
+ sock = skops->sk;
- if (!verdict || !parse)
- return -ENOENT;
+ /* 1. If sock map has BPF programs those will be inherited by the
+ * sock being added. If the sock is already attached to BPF programs
+ * this results in an error.
+ */
+ verdict = READ_ONCE(stab->bpf_verdict);
+ parse = READ_ONCE(stab->bpf_parse);
+ if (parse && verdict) {
/* bpf prog refcnt may be zero if a concurrent attach operation
* removes the program after the above READ_ONCE() but before
* we increment the refcnt. If this is the case abort with an
@@ -676,50 +711,78 @@ static int sock_map_ctx_update_elem(struct bpf_sock_ops_kern *skops,
}
}
- if (!psock) {
- sock = skops->sk;
- if (rcu_dereference_sk_user_data(sock))
- return -EEXIST;
+ write_lock_bh(&sock->sk_callback_lock);
+ psock = smap_psock_sk(sock);
+
+ /* 2. Do not allow inheriting programs if psock exists and has
+ * already inherited programs. This would create confusion on
+ * which parser/verdict program is running. If no psock exists
+ * create one. Inside sk_callback_lock to ensure concurrent create
+ * doesn't update user data.
+ */
+ if (psock) {
+ if (READ_ONCE(psock->bpf_parse) && parse) {
+ err = -EBUSY;
+ goto out_progs;
+ }
+ psock->refcnt++;
+ } else {
psock = smap_init_psock(sock, stab);
if (IS_ERR(psock)) {
- if (verdict)
- bpf_prog_put(verdict);
- if (parse)
- bpf_prog_put(parse);
- return PTR_ERR(psock);
+ err = PTR_ERR(psock);
+ goto out_progs;
}
- psock->key = i;
- psock->stab = stab;
- refcount_inc(&stab->refcnt);
+
set_bit(SMAP_TX_RUNNING, &psock->state);
}
- if (map_flags & BPF_SOCKMAP_STRPARSER) {
- write_lock_bh(&sock->sk_callback_lock);
- if (psock->strp_enabled)
- goto start_done;
+ e = kzalloc(sizeof(*e), GFP_ATOMIC | __GFP_NOWARN);
+ if (!e) {
+ err = -ENOMEM;
+ goto out_progs;
+ }
+ e->entry = &stab->sock_map[i];
+
+ /* 3. At this point we have a reference to a valid psock that is
+ * running. Attach any BPF programs needed.
+ */
+ if (parse && verdict && !psock->strp_enabled) {
err = smap_init_sock(psock, sock);
if (err)
- goto out;
+ goto out_free;
smap_init_progs(psock, stab, verdict, parse);
smap_start_sock(psock, sock);
-start_done:
- write_unlock_bh(&sock->sk_callback_lock);
- } else if (update) {
- smap_stop_sock(psock, sock);
}
- if (!update) {
- old_sock = xchg(&stab->sock_map[i], skops->sk);
- if (old_sock)
- smap_release_sock(old_sock);
- }
+ /* 4. Place psock in sockmap for use and stop any programs on
+ * the old sock assuming its not the same sock we are replacing
+ * it with. Because we can only have a single set of programs if
+ * old_sock has a strp we can stop it.
+ */
+ list_add_tail(&e->list, &psock->maps);
+ write_unlock_bh(&sock->sk_callback_lock);
+ osock = xchg(&stab->sock_map[i], sock);
+ if (osock) {
+ struct smap_psock *opsock = smap_psock_sk(osock);
+
+ write_lock_bh(&osock->sk_callback_lock);
+ if (osock != sock && parse)
+ smap_stop_sock(opsock, osock);
+ smap_list_remove(opsock, &stab->sock_map[i]);
+ smap_release_sock(opsock, osock);
+ write_unlock_bh(&osock->sk_callback_lock);
+ }
return 0;
-out:
+out_free:
+ smap_release_sock(psock, sock);
+out_progs:
+ if (verdict)
+ bpf_prog_put(verdict);
+ if (parse)
+ bpf_prog_put(parse);
write_unlock_bh(&sock->sk_callback_lock);
- if (!update)
- smap_release_sock(sock);
+ kfree(e);
return err;
}
@@ -768,8 +831,7 @@ static int sock_map_update_elem(struct bpf_map *map,
return -EINVAL;
}
- err = sock_map_ctx_update_elem(&skops, map, key,
- flags, BPF_SOCKMAP_STRPARSER);
+ err = sock_map_ctx_update_elem(&skops, map, key, flags);
fput(socket->file);
return err;
}
@@ -783,11 +845,11 @@ static int sock_map_update_elem(struct bpf_map *map,
.map_delete_elem = sock_map_delete_elem,
};
-BPF_CALL_5(bpf_sock_map_update, struct bpf_sock_ops_kern *, bpf_sock,
- struct bpf_map *, map, void *, key, u64, flags, u64, map_flags)
+BPF_CALL_4(bpf_sock_map_update, struct bpf_sock_ops_kern *, bpf_sock,
+ struct bpf_map *, map, void *, key, u64, flags)
{
WARN_ON_ONCE(!rcu_read_lock_held());
- return sock_map_ctx_update_elem(bpf_sock, map, key, flags, map_flags);
+ return sock_map_ctx_update_elem(bpf_sock, map, key, flags);
}
const struct bpf_func_proto bpf_sock_map_update_proto = {
@@ -799,5 +861,4 @@ static int sock_map_update_elem(struct bpf_map *map,
.arg2_type = ARG_CONST_MAP_PTR,
.arg3_type = ARG_PTR_TO_MAP_KEY,
.arg4_type = ARG_ANYTHING,
- .arg5_type = ARG_ANYTHING,
};
^ 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