* Re: [PATCH net-next] net: phy: use of_mdio_parse_addr
From: David Miller @ 2017-06-02 18:22 UTC (permalink / raw)
To: jon.mason
Cc: andrew, f.fainelli, netdev, linux-kernel,
bcm-kernel-feedback-list
In-Reply-To: <1496259810-3602-1-git-send-email-jon.mason@broadcom.com>
From: Jon Mason <jon.mason@broadcom.com>
Date: Wed, 31 May 2017 15:43:30 -0400
> use of_mdio_parse_addr() in place of an OF read of reg and a bounds
> check (which is litterally the exact same thing that
> of_mdio_parse_addr() does)
>
> Signed-off-by: Jon Mason <jon.mason@broadcom.com>
Applied, thanks Jon.
^ permalink raw reply
* Re: [PATCH net-next v2 1/2] bpf: Allow CGROUP_SKB eBPF program to access sk_buff
From: David Miller @ 2017-06-02 18:24 UTC (permalink / raw)
To: chenbofeng.kernel; +Cc: netdev, lorenzo, fengc
In-Reply-To: <1496279760-20996-1-git-send-email-chenbofeng.kernel@gmail.com>
From: Chenbo Feng <chenbofeng.kernel@gmail.com>
Date: Wed, 31 May 2017 18:15:59 -0700
> From: Chenbo Feng <fengc@google.com>
>
> This allows cgroup eBPF program to classify packet based on their
> protocol or other detail information. Currently program need
> CAP_NET_ADMIN privilege to attach a cgroup eBPF program, and A
> process with CAP_NET_ADMIN can already see all packets on the system,
> for example, by creating an iptables rules that causes the packet to
> be passed to userspace via NFLOG.
>
> Signed-off-by: Chenbo Feng <fengc@google.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next v2 2/2] bpf: Remove the capability check for cgroup skb eBPF program
From: David Miller @ 2017-06-02 18:25 UTC (permalink / raw)
To: chenbofeng.kernel; +Cc: netdev, lorenzo, fengc
In-Reply-To: <1496279760-20996-2-git-send-email-chenbofeng.kernel@gmail.com>
From: Chenbo Feng <chenbofeng.kernel@gmail.com>
Date: Wed, 31 May 2017 18:16:00 -0700
> From: Chenbo Feng <fengc@google.com>
>
> Currently loading a cgroup skb eBPF program require a CAP_SYS_ADMIN
> capability while attaching the program to a cgroup only requires the
> user have CAP_NET_ADMIN privilege. We can escape the capability
> check when load the program just like socket filter program to make
> the capability requirement consistent.
>
> Change since v1:
> Change the code style in order to be compliant with checkpatch.pl
> preference
>
> Signed-off-by: Chenbo Feng <fengc@google.com>
Applied.
^ permalink raw reply
* Re: Leak in ipv6_gso_segment()?
From: Craig Gallek @ 2017-06-02 18:25 UTC (permalink / raw)
To: David Miller; +Cc: Ben Hutchings, netdev
In-Reply-To: <20170602.140506.1833605091602891315.davem@davemloft.net>
On Fri, Jun 2, 2017 at 2:05 PM, David Miller <davem@davemloft.net> wrote:
> From: Ben Hutchings <ben@decadent.org.uk>
> Date: Wed, 31 May 2017 13:26:02 +0100
>
>> If I'm not mistaken, ipv6_gso_segment() now leaks segs if
>> ip6_find_1stfragopt() fails. I'm not sure whether the fix would be as
>> simple as adding a kfree_skb(segs) or whether more complex cleanup is
>> needed.
>
> I think we need to use kfree_skb_list(), like the following.
I think this is problematic as well. ipv6_gso_segment could
previously return errors, in which case the caller uses kfree_skb (ex
validate_xmit_skb() -> skb_gso_segment -> ...
callbacks.gso_segment()). Having the kfree_skb_list here would cause
a double free if I'm reading this correctly.
My first guess was going to be skb_gso_error_unwind(), but I'm still
trying to understand that code...
Sorry again for the fallout from this bug fix. This is my first time
down this code path and I clearly didn't understand it fully :/
> Can someone please audit and review this for me? We need to
> get this to -stable.
>
> Thanks.
>
> diff --git a/net/ipv6/ip6_offload.c b/net/ipv6/ip6_offload.c
> index 280268f..cdb3728 100644
> --- a/net/ipv6/ip6_offload.c
> +++ b/net/ipv6/ip6_offload.c
> @@ -116,8 +116,10 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff *skb,
>
> if (udpfrag) {
> int err = ip6_find_1stfragopt(skb, &prevhdr);
> - if (err < 0)
> + if (err < 0) {
> + kfree_skb_list(segs);
> return ERR_PTR(err);
> + }
> fptr = (struct frag_hdr *)((u8 *)ipv6h + err);
> fptr->frag_off = htons(offset);
> if (skb->next)
^ permalink raw reply
* Re: [PATCH net v2] ibmvnic: Remove module author mailing address
From: David Miller @ 2017-06-02 18:27 UTC (permalink / raw)
To: tlfalcon; +Cc: netdev, nfont, jallen
In-Reply-To: <1496349154-26479-1-git-send-email-tlfalcon@linux.vnet.ibm.com>
From: Thomas Falcon <tlfalcon@linux.vnet.ibm.com>
Date: Thu, 1 Jun 2017 15:32:34 -0500
> The original author left the project and so far has not
> responded to emails sent to the listed address.
>
> Signed-off-by: John Allen <jallen@linux.vnet.ibm.com>
> Signed-off-by: Thomas Falcon <tlfalcon@linux.vnet.ibm.com>
> ---
> v2: only remove invalid email address
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next] tcp: remove unnecessary skb_reset_tail_pointer()
From: David Miller @ 2017-06-02 18:28 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <1496351916.27480.23.camel@edumazet-glaptop3.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 01 Jun 2017 14:18:36 -0700
> From: Eric Dumazet <edumazet@google.com>
>
> __pskb_trim_head() does not need to reset skb tail pointer.
>
> Also change the comments, __pskb_pull_head() does not exist.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Applied, thanks Eric.
^ permalink raw reply
* Re: [net,V2] vxlan: fix use-after-free on deletion
From: David Miller @ 2017-06-02 18:30 UTC (permalink / raw)
To: markb; +Cc: jbenc, roid, roopa, netdev
In-Reply-To: <1496363048-21138-1-git-send-email-markb@mellanox.com>
From: Mark Bloch <markb@mellanox.com>
Date: Fri, 2 Jun 2017 03:24:08 +0300
> Adding a vxlan interface to a socket isn't symmetrical, while adding
> is done in vxlan_open() the deletion is done in vxlan_dellink().
> This can cause a use-after-free error when we close the vxlan
> interface before deleting it.
>
> We add vxlan_vs_del_dev() to match vxlan_vs_add_dev() and call
> it from vxlan_stop() to match the call from vxlan_open().
>
> Fixes: 56ef9c909b40 ("vxlan: Move socket initialization to within rtnl scope")
> Acked-by: Jiri Benc <jbenc@redhat.com>
> Tested-by: Roi Dayan <roid@mellanox.com>
> Signed-off-by: Mark Bloch <markb@mellanox.com>
> ---
> V2: Added a Fixes tag.
> Added Acked-by and Tested-by from Jiri and Roi.
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH net 1/1] qlcnic: Fix tunnel offload for 82xx adapters
From: David Miller @ 2017-06-02 18:31 UTC (permalink / raw)
To: manish.chopra; +Cc: netdev, Dept-GELinuxNICDev
In-Reply-To: <20170602135254.3591-1-manish.chopra@cavium.com>
From: Manish Chopra <manish.chopra@cavium.com>
Date: Fri, 2 Jun 2017 06:52:54 -0700
> Qlogic's 82xx series adapter doesn't support
> tunnel offloads, driver incorrectly assumes that it is
> supported and causes firmware hang while running tunnel IO.
>
> This patch fixes this by not advertising tunnel offloads
> for 82xx adapters.
>
> Signed-off-by: Manish Chopra <manish.chopra@cavium.com>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH v2] virtio_net: lower limit on buffer size
From: David Miller @ 2017-06-02 18:32 UTC (permalink / raw)
To: mst; +Cc: netdev, linux-kernel, virtualization, bfields, mpatocka
In-Reply-To: <1496415192-17031-1-git-send-email-mst@redhat.com>
From: "Michael S. Tsirkin" <mst@redhat.com>
Date: Fri, 2 Jun 2017 17:54:33 +0300
> commit d85b758f72b0 ("virtio_net: fix support for small rings")
> was supposed to increase the buffer size for small rings but had an
> unintentional side effect of decreasing it for large rings. This seems
> to break some setups - it's not yet clear why, but increasing buffer
> size back to what it was before helps.
>
> Fixes: d85b758f72b0 ("virtio_net: fix support for small rings")
> Reported-by: Mikulas Patocka <mpatocka@redhat.com>
> Reported-by: "J. Bruce Fields" <bfields@fieldses.org>
> Tested-by: Mikulas Patocka <mpatocka@redhat.com>
> Tested-by: "J. Bruce Fields" <bfields@fieldses.org>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>
> Patch tested, pls merge.
>
> changes from v2:
> commit log tweaks to address comments by Sergei Shtylyov
Applied, thanks Michael.
^ permalink raw reply
* Re: Cherry pick commit *net: better skb->sender_cpu and skb->napi_id cohabitation* to 4.4 series?
From: David Miller @ 2017-06-02 18:37 UTC (permalink / raw)
To: xiyou.wangcong; +Cc: pmenzel, edumazet, netdev, ast, it+linux-netdev, gregkh
In-Reply-To: <CAM_iQpXg3XCyfi7kStkWOh3R4+0yQHN15MOZ2dkbEYZvR1Xf2Q@mail.gmail.com>
From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Fri, 2 Jun 2017 10:51:39 -0700
> On Thu, Jun 1, 2017 at 10:32 AM, Paul Menzel <pmenzel@molgen.mpg.de> wrote:
>> Dear David,
>>
>>
>> On 06/01/17 18:19, David Miller wrote:
>>>
>>> Neither of us have anything to do with the 4.4 -stable series. We only
>>> manage stable backport submissions for the most recent two releases.
>>
>>
>> Thank you for the prompt response. Should the netdev FAQ be updated then to
>> contain that information?
>
> +1
>
> It is quite helpful to know which stable releases contain network fixes
> which don't, especially for those who rely on stable releases, at least
> we could do backport by ourselves when we know they are missing.
The people who maintain the -stable releases older than the ones Greg KH
are doing typically scan the ones that Greg applies and tries to integrate
them into their -stable releases when appropriate.
If someone wants to write an accurate description of that and how it works,
and add it to the FAQ, sure.
^ permalink raw reply
* Re: [PATCH V5 net-next] neigh: Really delete an arp/neigh entry on "ip neigh delete" or "arp -d"
From: Julian Anastasov @ 2017-06-02 18:40 UTC (permalink / raw)
To: Sowmini Varadhan; +Cc: netdev, davem, stephen
In-Reply-To: <1496419309-91478-1-git-send-email-sowmini.varadhan@oracle.com>
Hello,
On Fri, 2 Jun 2017, Sowmini Varadhan wrote:
> The command
> # arp -s 62.2.0.1 a:b:c:d:e:f dev eth2
> adds an entry like the following (listed by "arp -an")
> ? (62.2.0.1) at 0a:0b:0c:0d:0e:0f [ether] PERM on eth2
> but the symmetric deletion command
> # arp -i eth2 -d 62.2.0.1
> does not remove the PERM entry from the table, and instead leaves behind
> ? (62.2.0.1) at <incomplete> on eth2
>
> The reason is that there is a refcnt of 1 for the arp_tbl itself
> (neigh_alloc starts off the entry with a refcnt of 1), thus
> the neigh_release() call from arp_invalidate() will (at best) just
> decrement the ref to 1, but will never actually free it from the
> table.
>
> To fix this, we need to do something like neigh_forced_gc: if
> the refcnt is 1 (i.e., on the table's ref), remove the entry from
> the table and free it. This patch refactors and shares common code
> between neigh_forced_gc and the newly added neigh_remove_one.
>
> A similar issue exists for IPv6 Neighbor Cache entries, and is fixed
> in a similar manner by this patch.
>
> Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
> Reviewed-by: Julian Anastasov <ja@ssi.bg>
Very good, thanks!
> ---
> v2: kbuild-test-robot compile error
> v3: do not export_symbol neigh_remove_one (David Miller comment)
> v4: rearrange locking for tbl->lock (Julian Anastasov comment)
> v5: minor touch-ups: removed retval from neigh_remove_one (Julian Anastasov),
> got rid of checkpatch warnings, explicitly add net-next to subject line.
>
> include/net/neighbour.h | 1 +
> net/core/neighbour.c | 60 ++++++++++++++++++++++++++++++++++++++--------
> net/ipv4/arp.c | 4 +++
> 3 files changed, 54 insertions(+), 11 deletions(-)
>
> diff --git a/include/net/neighbour.h b/include/net/neighbour.h
> index e4dd3a2..639b675 100644
> --- a/include/net/neighbour.h
> +++ b/include/net/neighbour.h
> @@ -317,6 +317,7 @@ struct neighbour *__neigh_create(struct neigh_table *tbl, const void *pkey,
> int neigh_update(struct neighbour *neigh, const u8 *lladdr, u8 new, u32 flags,
> u32 nlmsg_pid);
> void __neigh_set_probe_once(struct neighbour *neigh);
> +bool neigh_remove_one(struct neighbour *ndel, struct neigh_table *tbl);
> void neigh_changeaddr(struct neigh_table *tbl, struct net_device *dev);
> int neigh_ifdown(struct neigh_table *tbl, struct net_device *dev);
> int neigh_resolve_output(struct neighbour *neigh, struct sk_buff *skb);
> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> index d274f81..dadb5ee 100644
> --- a/net/core/neighbour.c
> +++ b/net/core/neighbour.c
> @@ -118,6 +118,50 @@ unsigned long neigh_rand_reach_time(unsigned long base)
> EXPORT_SYMBOL(neigh_rand_reach_time);
>
>
> +static bool neigh_del(struct neighbour *n, __u8 state,
> + struct neighbour __rcu **np, struct neigh_table *tbl)
> +{
> + bool retval = false;
> +
> + write_lock(&n->lock);
> + if (atomic_read(&n->refcnt) == 1 && !(n->nud_state & state)) {
> + struct neighbour *neigh;
> +
> + neigh = rcu_dereference_protected(n->next,
> + lockdep_is_held(&tbl->lock));
> + rcu_assign_pointer(*np, neigh);
> + n->dead = 1;
> + retval = true;
> + }
> + write_unlock(&n->lock);
> + if (retval)
> + neigh_cleanup_and_release(n);
> + return retval;
> +}
> +
> +bool neigh_remove_one(struct neighbour *ndel, struct neigh_table *tbl)
> +{
> + struct neigh_hash_table *nht;
> + void *pkey = ndel->primary_key;
> + u32 hash_val;
> + struct neighbour *n;
> + struct neighbour __rcu **np;
> +
> + nht = rcu_dereference_protected(tbl->nht,
> + lockdep_is_held(&tbl->lock));
> + hash_val = tbl->hash(pkey, ndel->dev, nht->hash_rnd);
> + hash_val = hash_val >> (32 - nht->hash_shift);
> +
> + np = &nht->hash_buckets[hash_val];
> + while ((n = rcu_dereference_protected(*np,
> + lockdep_is_held(&tbl->lock)))) {
> + if (n == ndel)
> + return neigh_del(n, 0, np, tbl);
> + np = &n->next;
> + }
> + return false;
> +}
> +
> static int neigh_forced_gc(struct neigh_table *tbl)
> {
> int shrunk = 0;
> @@ -140,19 +184,10 @@ static int neigh_forced_gc(struct neigh_table *tbl)
> * - nobody refers to it.
> * - it is not permanent
> */
> - write_lock(&n->lock);
> - if (atomic_read(&n->refcnt) == 1 &&
> - !(n->nud_state & NUD_PERMANENT)) {
> - rcu_assign_pointer(*np,
> - rcu_dereference_protected(n->next,
> - lockdep_is_held(&tbl->lock)));
> - n->dead = 1;
> - shrunk = 1;
> - write_unlock(&n->lock);
> - neigh_cleanup_and_release(n);
> + if (neigh_del(n, NUD_PERMANENT, np, tbl)) {
> + shrunk = 1;
> continue;
> }
> - write_unlock(&n->lock);
> np = &n->next;
> }
> }
> @@ -1649,7 +1684,10 @@ static int neigh_delete(struct sk_buff *skb, struct nlmsghdr *nlh,
> NEIGH_UPDATE_F_OVERRIDE |
> NEIGH_UPDATE_F_ADMIN,
> NETLINK_CB(skb).portid);
> + write_lock_bh(&tbl->lock);
> neigh_release(neigh);
> + neigh_remove_one(neigh, tbl);
> + write_unlock_bh(&tbl->lock);
>
> out:
> return err;
> diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
> index e9f3386..a651c53 100644
> --- a/net/ipv4/arp.c
> +++ b/net/ipv4/arp.c
> @@ -1113,13 +1113,17 @@ static int arp_invalidate(struct net_device *dev, __be32 ip)
> {
> struct neighbour *neigh = neigh_lookup(&arp_tbl, &ip, dev);
> int err = -ENXIO;
> + struct neigh_table *tbl = &arp_tbl;
>
> if (neigh) {
> if (neigh->nud_state & ~NUD_NOARP)
> err = neigh_update(neigh, NULL, NUD_FAILED,
> NEIGH_UPDATE_F_OVERRIDE|
> NEIGH_UPDATE_F_ADMIN, 0);
> + write_lock_bh(&tbl->lock);
> neigh_release(neigh);
> + neigh_remove_one(neigh, tbl);
> + write_unlock_bh(&tbl->lock);
> }
>
> return err;
> --
> 1.7.1
Regards
^ permalink raw reply
* Re: [PATCH] brcmfmac: Fix glob_skb leak in brcmf_sdiod_recv_chain
From: Florian Fainelli @ 2017-06-02 18:48 UTC (permalink / raw)
To: Peter S. Housel, Arend van Spriel, Franky Lin, Hante Meuleman,
Kalle Valo, Christian Daudt, Pieter-Paul Giesberts,
Martin Blumenstingl,
open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER,
open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER,
open list:NETWORKING DRIVERS, open list
In-Reply-To: <1496424146-10928-1-git-send-email-housel@acm.org>
On 06/02/2017 10:22 AM, Peter S. Housel wrote:
> An earlier change to this function (3bdae810721b) fixed a leak in the
> case of an unsuccessful call to brcmf_sdiod_buffrw(). However, the
> glob_skb buffer, used for emulating a scattering read, is never used
> or referenced after its contents are copied into the destination
> buffers, and therefore always needs to be freed by the end of the
> function.
That looks correct, could you add the relevant Fixes tag for this?
Fixes: 3bdae810721b ("brcmfmac: Fix glob_skb leak in
brcmf_sdiod_recv_chain")
Fixes: a413e39a38573 ("brcmfmac: fix brcmf_sdcard_recv_chain() for host
without sg support")
BTW, you made the same typo that I did, it's actually glom_skb ;)
>
> Signed-off-by: Peter S. Housel <housel@acm.org>
> ---
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
> index 9b970dc..4c5064f 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
> @@ -727,15 +727,16 @@ int brcmf_sdiod_recv_chain(struct brcmf_sdio_dev *sdiodev,
> return -ENOMEM;
> err = brcmf_sdiod_buffrw(sdiodev, SDIO_FUNC_2, false, addr,
> glom_skb);
> - if (err) {
> - brcmu_pkt_buf_free_skb(glom_skb);
> - goto done;
> - }
> + if (err)
> + goto free_glom_skb;
>
> skb_queue_walk(pktq, skb) {
> memcpy(skb->data, glom_skb->data, skb->len);
> skb_pull(glom_skb, skb->len);
> }
> +
> +free_glom_skb:
> + brcmu_pkt_buf_free_skb(glom_skb);
> } else
> err = brcmf_sdiod_sglist_rw(sdiodev, SDIO_FUNC_2, false, addr,
> pktq);
>
--
Florian
^ permalink raw reply
* Re: [PATCH] brcmfmac: Fix glob_skb leak in brcmf_sdiod_recv_chain
From: Franky Lin @ 2017-06-02 18:52 UTC (permalink / raw)
To: Peter S. Housel
Cc: Arend van Spriel, Hante Meuleman, Kalle Valo, Christian Daudt,
Pieter-Paul Giesberts, Martin Blumenstingl, Florian Fainelli,
open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER,
open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER,
open list:NETWORKING DRIVERS, open list
In-Reply-To: <1496424146-10928-1-git-send-email-housel@acm.org>
On Fri, Jun 2, 2017 at 10:22 AM, Peter S. Housel <housel@acm.org> wrote:
> An earlier change to this function (3bdae810721b) fixed a leak in the
> case of an unsuccessful call to brcmf_sdiod_buffrw(). However, the
> glob_skb buffer, used for emulating a scattering read, is never used
> or referenced after its contents are copied into the destination
> buffers, and therefore always needs to be freed by the end of the
> function.
>
> Signed-off-by: Peter S. Housel <housel@acm.org>
> ---
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
> index 9b970dc..4c5064f 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
> @@ -727,15 +727,16 @@ int brcmf_sdiod_recv_chain(struct brcmf_sdio_dev *sdiodev,
> return -ENOMEM;
> err = brcmf_sdiod_buffrw(sdiodev, SDIO_FUNC_2, false, addr,
> glom_skb);
> - if (err) {
> - brcmu_pkt_buf_free_skb(glom_skb);
> - goto done;
> - }
> + if (err)
> + goto free_glom_skb;
>
> skb_queue_walk(pktq, skb) {
> memcpy(skb->data, glom_skb->data, skb->len);
> skb_pull(glom_skb, skb->len);
> }
> +
> +free_glom_skb:
> + brcmu_pkt_buf_free_skb(glom_skb);
What about
if (!err) {
skb_queue_walk(pktq, skb) {
memcpy(skb->data, glom_skb->data, skb->len);
skb_pull(glom_skb, skb->len);
}
}
brcmu_pkt_buf_free_skb(glom_skb);
Then no goto is needed.
Thanks,
Franky
> } else
> err = brcmf_sdiod_sglist_rw(sdiodev, SDIO_FUNC_2, false, addr,
> pktq);
> --
> 2.7.4
>
^ permalink raw reply
* [PATCH net] geneve: fix needed_headroom and max_mtu for collect_metadata
From: Eric Garver @ 2017-06-02 18:54 UTC (permalink / raw)
To: David Miller, pravin shelar; +Cc: netdev
Since commit 9b4437a5b870 ("geneve: Unify LWT and netdev handling.")
when using COLLECT_METADATA geneve devices are created with too small of
a needed_headroom and too large of a max_mtu. This is because
ip_tunnel_info_af() is not valid with the device level info when using
COLLECT_METADATA and we mistakenly fall into the IPv4 case.
For COLLECT_METADATA, always use the worst case of ipv6 since both
sockets are created.
Fixes: 9b4437a5b870 ("geneve: Unify LWT and netdev handling.")
Signed-off-by: Eric Garver <e@erig.me>
---
drivers/net/geneve.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
index 959fd12d2e67..6ebb0f559a42 100644
--- a/drivers/net/geneve.c
+++ b/drivers/net/geneve.c
@@ -1133,7 +1133,7 @@ static int geneve_configure(struct net *net, struct net_device *dev,
/* make enough headroom for basic scenario */
encap_len = GENEVE_BASE_HLEN + ETH_HLEN;
- if (ip_tunnel_info_af(info) == AF_INET) {
+ if (!metadata && ip_tunnel_info_af(info) == AF_INET) {
encap_len += sizeof(struct iphdr);
dev->max_mtu -= sizeof(struct iphdr);
} else {
--
2.12.0
^ permalink raw reply related
* Re: Leak in ipv6_gso_segment()?
From: Craig Gallek @ 2017-06-02 19:27 UTC (permalink / raw)
To: David Miller; +Cc: Ben Hutchings, netdev
In-Reply-To: <CAEfhGizesRT9YOfKs09mbnA+PKnG5i2ERmPNyx_+Z6LZ1_bcFQ@mail.gmail.com>
On Fri, Jun 2, 2017 at 2:25 PM, Craig Gallek <kraigatgoog@gmail.com> wrote:
> On Fri, Jun 2, 2017 at 2:05 PM, David Miller <davem@davemloft.net> wrote:
>> From: Ben Hutchings <ben@decadent.org.uk>
>> Date: Wed, 31 May 2017 13:26:02 +0100
>>
>>> If I'm not mistaken, ipv6_gso_segment() now leaks segs if
>>> ip6_find_1stfragopt() fails. I'm not sure whether the fix would be as
>>> simple as adding a kfree_skb(segs) or whether more complex cleanup is
>>> needed.
>>
>> I think we need to use kfree_skb_list(), like the following.
> I think this is problematic as well. ipv6_gso_segment could
> previously return errors, in which case the caller uses kfree_skb (ex
> validate_xmit_skb() -> skb_gso_segment -> ...
> callbacks.gso_segment()). Having the kfree_skb_list here would cause
> a double free if I'm reading this correctly.
>
> My first guess was going to be skb_gso_error_unwind(), but I'm still
> trying to understand that code...
>
> Sorry again for the fallout from this bug fix. This is my first time
> down this code path and I clearly didn't understand it fully :/
Ok, I take it back. I believe your kfree_skb_list suggestion is correct.
I was assuming that skb_segment consumed the original skb upon
successful segmentation. It does not. This is exactly why
validate_xmit_skb calls consume_skb when segments are returned.
Further, there is at least one existing example of kfree_skb_list in a
similar post-semgent cleanup path (esp6_gso_segment).
^ permalink raw reply
* [PATCH 0/3] net: dsa: Cleanups before multi-CPU port
From: Florian Fainelli @ 2017-06-02 19:31 UTC (permalink / raw)
To: netdev; +Cc: andrew, vivien.didelot, john, Florian Fainelli
Hi all,
This patch series does a bunch of cleanups before we start adding support
for multiple CPU ports.
Florian Fainelli (3):
net: dsa: Pass dsa_port reference to ethtool setup/restore
net: dsa: Consistently use dsa_port for dsa_*_port_{apply,unapply}
net: dsa: Initialize all CPU and enabled ports masks in dsa_ds_parse()
include/net/dsa.h | 2 +-
net/dsa/dsa.c | 6 ++-
net/dsa/dsa2.c | 125 ++++++++++++++++++++++++-----------------------------
net/dsa/dsa_priv.h | 4 +-
net/dsa/legacy.c | 4 +-
5 files changed, 66 insertions(+), 75 deletions(-)
--
2.9.3
^ permalink raw reply
* [PATCH 1/3] net: dsa: Pass dsa_port reference to ethtool setup/restore
From: Florian Fainelli @ 2017-06-02 19:31 UTC (permalink / raw)
To: netdev; +Cc: andrew, vivien.didelot, john, Florian Fainelli
In-Reply-To: <20170602193123.23930-1-f.fainelli@gmail.com>
We do not need to have a reference to a dsa_switch, instead we should
pass a reference to a CPU dsa_port, change that. This is a preliminary
change to better support multiple CPU ports.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
net/dsa/dsa.c | 6 ++++--
net/dsa/dsa2.c | 4 ++--
net/dsa/dsa_priv.h | 4 ++--
net/dsa/legacy.c | 4 ++--
4 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 402459e73f33..fdc448b30e56 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -112,8 +112,9 @@ const struct dsa_device_ops *dsa_resolve_tag_protocol(int tag_protocol)
return ops;
}
-int dsa_cpu_port_ethtool_setup(struct dsa_switch *ds)
+int dsa_cpu_port_ethtool_setup(struct dsa_port *cpu_dp)
{
+ struct dsa_switch *ds = cpu_dp->ds;
struct net_device *master;
struct ethtool_ops *cpu_ops;
@@ -136,8 +137,9 @@ int dsa_cpu_port_ethtool_setup(struct dsa_switch *ds)
return 0;
}
-void dsa_cpu_port_ethtool_restore(struct dsa_switch *ds)
+void dsa_cpu_port_ethtool_restore(struct dsa_port *cpu_dp)
{
+ struct dsa_switch *ds = cpu_dp->ds;
struct net_device *master;
master = ds->dst->master_netdev;
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 21b44a9828f6..7f9bf1456a65 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -444,7 +444,7 @@ static int dsa_dst_apply(struct dsa_switch_tree *dst)
}
if (dst->cpu_dp) {
- err = dsa_cpu_port_ethtool_setup(dst->cpu_dp->ds);
+ err = dsa_cpu_port_ethtool_setup(dst->cpu_dp);
if (err)
return err;
}
@@ -485,7 +485,7 @@ static void dsa_dst_unapply(struct dsa_switch_tree *dst)
}
if (dst->cpu_dp)
- dsa_cpu_port_ethtool_restore(dst->cpu_dp->ds);
+ dsa_cpu_port_ethtool_restore(dst->cpu_dp);
pr_info("DSA: tree %d unapplied\n", dst->tree);
dst->applied = false;
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index db2a7b9edfb8..66ee248796c8 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -103,8 +103,8 @@ int dsa_cpu_dsa_setup(struct dsa_switch *ds, struct device *dev,
struct dsa_port *dport, int port);
void dsa_cpu_dsa_destroy(struct dsa_port *dport);
const struct dsa_device_ops *dsa_resolve_tag_protocol(int tag_protocol);
-int dsa_cpu_port_ethtool_setup(struct dsa_switch *ds);
-void dsa_cpu_port_ethtool_restore(struct dsa_switch *ds);
+int dsa_cpu_port_ethtool_setup(struct dsa_port *cpu_dp);
+void dsa_cpu_port_ethtool_restore(struct dsa_port *cpu_dp);
/* legacy.c */
int dsa_legacy_register(void);
diff --git a/net/dsa/legacy.c b/net/dsa/legacy.c
index d70a1a788d17..d534d8f4b9cf 100644
--- a/net/dsa/legacy.c
+++ b/net/dsa/legacy.c
@@ -205,7 +205,7 @@ static int dsa_switch_setup_one(struct dsa_switch *ds, struct device *parent)
netdev_err(dst->master_netdev, "[%d] : can't configure CPU and DSA ports\n",
index);
- ret = dsa_cpu_port_ethtool_setup(ds);
+ ret = dsa_cpu_port_ethtool_setup(ds->dst->cpu_dp);
if (ret)
return ret;
@@ -733,7 +733,7 @@ static void dsa_remove_dst(struct dsa_switch_tree *dst)
dsa_switch_destroy(ds);
}
- dsa_cpu_port_ethtool_restore(dst->cpu_dp->ds);
+ dsa_cpu_port_ethtool_restore(dst->cpu_dp);
dev_put(dst->master_netdev);
}
--
2.9.3
^ permalink raw reply related
* [PATCH 3/3] net: dsa: Initialize all CPU and enabled ports masks in dsa_ds_parse()
From: Florian Fainelli @ 2017-06-02 19:31 UTC (permalink / raw)
To: netdev; +Cc: andrew, vivien.didelot, john, Florian Fainelli
In-Reply-To: <20170602193123.23930-1-f.fainelli@gmail.com>
There was no reason for duplicating the code that initializes
ds->enabled_port_mask in both dsa_parse_ports_dn() and
dsa_parse_ports(), instead move this to dsa_ds_parse() which is early
enough before ops->setup() has run.
While at it, we can now make dsa_is_cpu_port() check ds->cpu_port_mask
which is a step towards being multi-CPU port capable.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
include/net/dsa.h | 2 +-
net/dsa/dsa2.c | 33 +++++++++++++++------------------
2 files changed, 16 insertions(+), 19 deletions(-)
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 448d8bc77707..2effb0af9d7c 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -254,7 +254,7 @@ struct dsa_switch {
static inline bool dsa_is_cpu_port(struct dsa_switch *ds, int p)
{
- return ds->dst->cpu_dp == &ds->ports[p];
+ return !!(ds->cpu_port_mask & (1 << p));
}
static inline bool dsa_is_dsa_port(struct dsa_switch *ds, int p)
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 067daec644c1..cd13bb54a30c 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -250,8 +250,6 @@ static int dsa_cpu_port_apply(struct dsa_port *port)
return err;
}
- ds->cpu_port_mask |= BIT(port->index);
-
memset(&port->devlink_port, 0, sizeof(port->devlink_port));
err = devlink_port_register(ds->devlink, &port->devlink_port,
port->index);
@@ -522,6 +520,12 @@ static int dsa_cpu_parse(struct dsa_port *port, u32 index,
dst->rcv = dst->tag_ops->rcv;
+ /* Initialize cpu_port_mask now for drv->setup()
+ * to have access to a correct value, just like what
+ * net/dsa/dsa.c::dsa_switch_setup_one does.
+ */
+ ds->cpu_port_mask |= BIT(index);
+
return 0;
}
@@ -533,14 +537,22 @@ static int dsa_ds_parse(struct dsa_switch_tree *dst, struct dsa_switch *ds)
for (index = 0; index < ds->num_ports; index++) {
port = &ds->ports[index];
- if (!dsa_port_is_valid(port))
+ if (!dsa_port_is_valid(port) ||
+ dsa_port_is_dsa(port))
continue;
if (dsa_port_is_cpu(port)) {
err = dsa_cpu_parse(port, index, dst, ds);
if (err)
return err;
+ } else {
+ /* Initialize enabled_port_mask now for drv->setup()
+ * to have access to a correct value, just like what
+ * net/dsa/dsa.c::dsa_switch_setup_one does.
+ */
+ ds->enabled_port_mask |= BIT(index);
}
+
}
pr_info("DSA: switch %d %d parsed\n", dst->tree, ds->index);
@@ -589,13 +601,6 @@ static int dsa_parse_ports_dn(struct device_node *ports, struct dsa_switch *ds)
return -EINVAL;
ds->ports[reg].dn = port;
-
- /* Initialize enabled_port_mask now for ops->setup()
- * to have access to a correct value, just like what
- * net/dsa/dsa.c::dsa_switch_setup_one does.
- */
- if (!dsa_port_is_cpu(&ds->ports[reg]))
- ds->enabled_port_mask |= 1 << reg;
}
return 0;
@@ -611,14 +616,6 @@ static int dsa_parse_ports(struct dsa_chip_data *cd, struct dsa_switch *ds)
continue;
ds->ports[i].name = cd->port_names[i];
-
- /* Initialize enabled_port_mask now for drv->setup()
- * to have access to a correct value, just like what
- * net/dsa/dsa.c::dsa_switch_setup_one does.
- */
- if (!dsa_port_is_cpu(&ds->ports[i]))
- ds->enabled_port_mask |= 1 << i;
-
valid_name_found = true;
}
--
2.9.3
^ permalink raw reply related
* [PATCH 2/3] net: dsa: Consistently use dsa_port for dsa_*_port_{apply,unapply}
From: Florian Fainelli @ 2017-06-02 19:31 UTC (permalink / raw)
To: netdev; +Cc: andrew, vivien.didelot, john, Florian Fainelli
In-Reply-To: <20170602193123.23930-1-f.fainelli@gmail.com>
We have all the information we need in dsa_port, so use it instead of
repeating the same arguments over and over again.
Suggested-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
net/dsa/dsa2.c | 90 ++++++++++++++++++++++++++--------------------------------
1 file changed, 41 insertions(+), 49 deletions(-)
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 7f9bf1456a65..067daec644c1 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -214,66 +214,61 @@ static int dsa_dst_complete(struct dsa_switch_tree *dst)
return 0;
}
-static int dsa_dsa_port_apply(struct dsa_port *port, u32 index,
- struct dsa_switch *ds)
+static int dsa_dsa_port_apply(struct dsa_port *port)
{
+ struct dsa_switch *ds = port->ds;
int err;
- err = dsa_cpu_dsa_setup(ds, ds->dev, port, index);
+ err = dsa_cpu_dsa_setup(ds, ds->dev, port, port->index);
if (err) {
dev_warn(ds->dev, "Failed to setup dsa port %d: %d\n",
- index, err);
+ port->index, err);
return err;
}
- memset(&ds->ports[index].devlink_port, 0,
- sizeof(ds->ports[index].devlink_port));
+ memset(&port->devlink_port, 0, sizeof(port->devlink_port));
- return devlink_port_register(ds->devlink,
- &ds->ports[index].devlink_port,
- index);
+ return devlink_port_register(ds->devlink, &port->devlink_port,
+ port->index);
}
-static void dsa_dsa_port_unapply(struct dsa_port *port, u32 index,
- struct dsa_switch *ds)
+static void dsa_dsa_port_unapply(struct dsa_port *port)
{
- devlink_port_unregister(&ds->ports[index].devlink_port);
+ devlink_port_unregister(&port->devlink_port);
dsa_cpu_dsa_destroy(port);
}
-static int dsa_cpu_port_apply(struct dsa_port *port, u32 index,
- struct dsa_switch *ds)
+static int dsa_cpu_port_apply(struct dsa_port *port)
{
+ struct dsa_switch *ds = port->ds;
int err;
- err = dsa_cpu_dsa_setup(ds, ds->dev, port, index);
+ err = dsa_cpu_dsa_setup(ds, ds->dev, port, port->index);
if (err) {
dev_warn(ds->dev, "Failed to setup cpu port %d: %d\n",
- index, err);
+ port->index, err);
return err;
}
- ds->cpu_port_mask |= BIT(index);
+ ds->cpu_port_mask |= BIT(port->index);
- memset(&ds->ports[index].devlink_port, 0,
- sizeof(ds->ports[index].devlink_port));
- err = devlink_port_register(ds->devlink, &ds->ports[index].devlink_port,
- index);
+ memset(&port->devlink_port, 0, sizeof(port->devlink_port));
+ err = devlink_port_register(ds->devlink, &port->devlink_port,
+ port->index);
return err;
}
-static void dsa_cpu_port_unapply(struct dsa_port *port, u32 index,
- struct dsa_switch *ds)
+static void dsa_cpu_port_unapply(struct dsa_port *port)
{
- devlink_port_unregister(&ds->ports[index].devlink_port);
+ devlink_port_unregister(&port->devlink_port);
dsa_cpu_dsa_destroy(port);
- ds->cpu_port_mask &= ~BIT(index);
+ port->ds->cpu_port_mask &= ~BIT(port->index);
}
-static int dsa_user_port_apply(struct dsa_port *port, u32 index,
- struct dsa_switch *ds)
+static int dsa_user_port_apply(struct dsa_port *port)
{
+ struct dsa_switch *ds = port->ds;
const char *name = port->name;
int err;
@@ -282,35 +277,32 @@ static int dsa_user_port_apply(struct dsa_port *port, u32 index,
if (!name)
name = "eth%d";
- err = dsa_slave_create(ds, ds->dev, index, name);
+ err = dsa_slave_create(ds, ds->dev, port->index, name);
if (err) {
dev_warn(ds->dev, "Failed to create slave %d: %d\n",
- index, err);
- ds->ports[index].netdev = NULL;
+ port->index, err);
+ port->netdev = NULL;
return err;
}
- memset(&ds->ports[index].devlink_port, 0,
- sizeof(ds->ports[index].devlink_port));
- err = devlink_port_register(ds->devlink, &ds->ports[index].devlink_port,
- index);
+ memset(&port->devlink_port, 0, sizeof(port->devlink_port));
+ err = devlink_port_register(ds->devlink, &port->devlink_port,
+ port->index);
if (err)
return err;
- devlink_port_type_eth_set(&ds->ports[index].devlink_port,
- ds->ports[index].netdev);
+ devlink_port_type_eth_set(&port->devlink_port, port->netdev);
return 0;
}
-static void dsa_user_port_unapply(struct dsa_port *port, u32 index,
- struct dsa_switch *ds)
+static void dsa_user_port_unapply(struct dsa_port *port)
{
- devlink_port_unregister(&ds->ports[index].devlink_port);
- if (ds->ports[index].netdev) {
- dsa_slave_destroy(ds->ports[index].netdev);
- ds->ports[index].netdev = NULL;
- ds->enabled_port_mask &= ~(1 << index);
+ devlink_port_unregister(&port->devlink_port);
+ if (port->netdev) {
+ dsa_slave_destroy(port->netdev);
+ port->netdev = NULL;
+ port->ds->enabled_port_mask &= ~(1 << port->index);
}
}
@@ -370,20 +362,20 @@ static int dsa_ds_apply(struct dsa_switch_tree *dst, struct dsa_switch *ds)
continue;
if (dsa_port_is_dsa(port)) {
- err = dsa_dsa_port_apply(port, index, ds);
+ err = dsa_dsa_port_apply(port);
if (err)
return err;
continue;
}
if (dsa_port_is_cpu(port)) {
- err = dsa_cpu_port_apply(port, index, ds);
+ err = dsa_cpu_port_apply(port);
if (err)
return err;
continue;
}
- err = dsa_user_port_apply(port, index, ds);
+ err = dsa_user_port_apply(port);
if (err)
continue;
}
@@ -402,16 +394,16 @@ static void dsa_ds_unapply(struct dsa_switch_tree *dst, struct dsa_switch *ds)
continue;
if (dsa_port_is_dsa(port)) {
- dsa_dsa_port_unapply(port, index, ds);
+ dsa_dsa_port_unapply(port);
continue;
}
if (dsa_port_is_cpu(port)) {
- dsa_cpu_port_unapply(port, index, ds);
+ dsa_cpu_port_unapply(port);
continue;
}
- dsa_user_port_unapply(port, index, ds);
+ dsa_user_port_unapply(port);
}
if (ds->slave_mii_bus && ds->ops->phy_read)
--
2.9.3
^ permalink raw reply related
* Re: [PATCH 1/3] net: dsa: Pass dsa_port reference to ethtool setup/restore
From: Vivien Didelot @ 2017-06-02 19:32 UTC (permalink / raw)
To: Florian Fainelli, netdev; +Cc: andrew, john, Florian Fainelli
In-Reply-To: <20170602193123.23930-2-f.fainelli@gmail.com>
Florian Fainelli <f.fainelli@gmail.com> writes:
> We do not need to have a reference to a dsa_switch, instead we should
> pass a reference to a CPU dsa_port, change that. This is a preliminary
> change to better support multiple CPU ports.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
^ permalink raw reply
* Re: [PATCH 2/3] net: dsa: Consistently use dsa_port for dsa_*_port_{apply,unapply}
From: Vivien Didelot @ 2017-06-02 19:32 UTC (permalink / raw)
To: Florian Fainelli, netdev; +Cc: andrew, john, Florian Fainelli
In-Reply-To: <20170602193123.23930-3-f.fainelli@gmail.com>
Florian Fainelli <f.fainelli@gmail.com> writes:
> We have all the information we need in dsa_port, so use it instead of
> repeating the same arguments over and over again.
>
> Suggested-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
^ permalink raw reply
* Re: [PATCH 3/3] net: dsa: Initialize all CPU and enabled ports masks in dsa_ds_parse()
From: Vivien Didelot @ 2017-06-02 19:33 UTC (permalink / raw)
To: Florian Fainelli, netdev; +Cc: andrew, john, Florian Fainelli
In-Reply-To: <20170602193123.23930-4-f.fainelli@gmail.com>
Florian Fainelli <f.fainelli@gmail.com> writes:
> There was no reason for duplicating the code that initializes
> ds->enabled_port_mask in both dsa_parse_ports_dn() and
> dsa_parse_ports(), instead move this to dsa_ds_parse() which is early
> enough before ops->setup() has run.
>
> While at it, we can now make dsa_is_cpu_port() check ds->cpu_port_mask
> which is a step towards being multi-CPU port capable.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
^ permalink raw reply
* Re: [PATCH 7/7] mlx5: Do not build eswitch_offloads if CONFIG_MLX5_EN_ESWITCH_OFFLOADS is set
From: Jes Sorensen @ 2017-06-02 20:22 UTC (permalink / raw)
To: Or Gerlitz, Jes Sorensen
Cc: Linux Netdev List, Kernel Team, Saeed Mahameed, Ilan Tayari
In-Reply-To: <CAJ3xEMh8ALo9Pgb0sh5dzAFaPtiKDaZyEhgasgi40Pba1JzCQA@mail.gmail.com>
On 05/28/2017 02:03 AM, Or Gerlitz wrote:
> On Sun, May 28, 2017 at 5:23 AM, Jes Sorensen <jes.sorensen@gmail.com> wrote:
>> On 05/27/2017 05:02 PM, Or Gerlitz wrote:
>>>
>>> On Sat, May 27, 2017 at 12:16 AM, Jes Sorensen <jes.sorensen@gmail.com>
>>> wrote:
>>>>
>>>> This gets rid of the temporary #ifdef spaghetti and allows the code to
>>>> compile without offload support enabled.
>
>>> I am pretty sure we can do that exercise you're up to without any
>>> spaghetti cooking and even put more code under that CONFIG directive
>>> (en_rep.c), I'll take that with Saeed.
>
>> I want to avoid adding #ifdef CONFIG_foo to the main code in order to keep
>> it readable. I did it gradually to make sure I didn't break anything and to
>> allow for it to be bisected in case something did break. If we can move out
>> more code from places like en_rep.c into eswitch_offload.c and get it
>> disabled that way that would be great, but I like to limit the number of
>> #ifdefs we add to the actual code.
>
> FWIW (see below), squashing your seven patches to one resulted in a
> fairly simple/clear
> patch, so if we go that way, no need to have seven commits just for this piece.
Squashing patches into jumbo patches is inherently broken and bad coding
practice! It makes it way more complicated to debug and bisect in case a
minor detail broke in the process.
>>> Just wondering, you are motivated by a wish to put some mlx5
>>> functionalities under their own CONFIG directives which could be
>>> useful when backporting the latest upstream driver into older kernel
>>> and being able not to deal with parts of it, right? in that respect,
>>> are you using SRIOV but not the offloads mode?
>
>> The motivation is two-fold, the primary is to be able to disable features
>> not being used for those who compile a custom kernel and who wish to reduce
>> the codebase compiled. It also makes it more flexible when back porting the
>> code to older kernels since it is easier to pick out a smaller subset. I was
>> going to look into making TC support etc. optional next, but I wanted to
>> have a discussion about this patchset first.
>
> OKay, I got you.
>
> Re SRIOV, I don't think it would be correct to break the support info few
> CONFIG directives. If we want to allow someone to build the driver w.o
> SRIOV that's fine, but I don't think we should further go down and disable
> some of the SRIOV sub-modes.
>
> Re TC offload support, that's make sense.
OK, so disabling SRIOV and disabling TC makes sense - I'll look at that.
Jes
^ permalink raw reply
* Re: [PATCH] virtio_net: lower limit on buffer size
From: J. Bruce Fields @ 2017-06-02 20:25 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: Michael S. Tsirkin, linux-kernel, Mikulas Patocka, Jason Wang,
virtualization, netdev
In-Reply-To: <3dd90d56-7110-bb9e-0b7f-4c8a8003a57f@cogentembedded.com>
On Fri, Jun 02, 2017 at 12:34:57PM +0300, Sergei Shtylyov wrote:
> Hello!
>
> On 6/2/2017 2:56 AM, Michael S. Tsirkin wrote:
>
> >commit d85b758f72b0 "virtio_net: fix support for small rings"
>
> Commit d85b758f72b0 ("virtio_net: fix support for small rings")
>
> >was supposed to increase the buffer size for small rings
> >but had an unintentional side effect of decreasing
> >it for large rings. This seems to break some setups -
> >it's not yet clear why, but increasing buffer size
> >back to what it was before helps.
> >
> >Fixes: d85b758f72b0 "virtio_net: fix support for small rings"
>
> Fixes: d85b758f72b0 ("virtio_net: fix support for small rings")
I may be bikeshedding, but, personally I never do the parens--they're
redundant given the quotes, and space is often tight.
--b.
>
> >Reported-by: Mikulas Patocka <mpatocka@redhat.com>
> >Reported-by: "J. Bruce Fields" <bfields@fieldses.org>
> >Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> [...]
>
> MBR, Sergei
^ permalink raw reply
* Re: [for-next 4/6] net/mlx5: FPGA, Add basic support for Innova
From: Jes Sorensen @ 2017-06-02 20:31 UTC (permalink / raw)
To: Ilan Tayari, Saeed Mahameed
Cc: Alexei Starovoitov, Saeed Mahameed, David S. Miller, Doug Ledford,
netdev@vger.kernel.org, linux-rdma@vger.kernel.org, Kernel Team
In-Reply-To: <AM4PR0501MB19406FF2555888457C00E217DBF20@AM4PR0501MB1940.eurprd05.prod.outlook.com>
On 05/28/2017 03:24 AM, Ilan Tayari wrote:
>> -----Original Message-----
>> From: Jes Sorensen [mailto:jsorensen@fb.com]
>>
>> On 05/26/2017 04:29 AM, Saeed Mahameed wrote:
>>> On Thu, May 25, 2017 at 11:48 PM, Jes Sorensen <jsorensen@fb.com> wrote:
>>>> On 05/25/2017 06:40 AM, Saeed Mahameed wrote:
>>> Hi Jes,
>>>
>>> No, It is clearly stated in the commit message :
>>>
>>> "The FPGA is a bump-on-the-wire and thus affects operation of
>>> the mlx5_core driver on the ConnectX ASIC."
>>>
>>> Which means mlx5 FPGA user can only write logic which affects only
>>> packets going in/out
>>> A ConnectX chip - so it is only network stuff -.
>>>
>>>> We have this with other devices in the kernel where a primary device
>> driver
>>>> provides an interface for an additional sub-driver to access another
>> device
>>>> behind it. Like bt-coexist in some of the wifi drivers allowing access
>> to a
>>>> bluetooth device behind it.
>>>
>>> Blutooth over wifi or vise versa is a very good example to what you
>>> are requesting.
>>> But, it doesn't fit to what we are trying to do here. mlx5 FGPA is a
>>> ConnectX card feature, not a new protocol.
>>
>> In that case it would need to be an optional module that can be enabled
>> or disabled at build time.
>
> Jes,
>
> It is already modular like that. See CONFIG_MLX5_FPGA.
[jes@xpeas netdev.git]$ grep CONFIG_MLX5_FPGA
drivers/net/ethernet/mellanox/mlx5/core/*
[jes@xpeas netdev.git]$
Which git tree am I supposed to look at?
Jes
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox