* Re: [PATCH net-next v2 3/9] net: phy: phylink: Poll link GPIOs
From: Russell King - ARM Linux @ 2018-05-10 20:29 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev, privat, andrew, vivien.didelot, davem, sean.wang,
Woojung.Huh, john, cphealy
In-Reply-To: <20180510201737.13887-4-f.fainelli@gmail.com>
On Thu, May 10, 2018 at 01:17:31PM -0700, Florian Fainelli wrote:
> From: Russell King <rmk+kernel@armlinux.org.uk>
>
> When using a fixed link with a link GPIO, we need to poll that GPIO to
> determine link state changes. This is consistent with what fixed_phy.c does.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
I'd like this to use the GPIO interrupt where available, only falling back
to the timer approach when there's no interrupt. Unfortunately, I don't
have much time to devote to this at the moment, having recently been away
on vacation, and now having to work on ARM specific issues for probably
all of the remainder of this kernel cycle.
That means I won't have time to test your series on any of the boards
I have available to me.
> ---
> drivers/net/phy/phylink.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
> index 6392b5248cf5..581ce93ecaf9 100644
> --- a/drivers/net/phy/phylink.c
> +++ b/drivers/net/phy/phylink.c
> @@ -19,6 +19,7 @@
> #include <linux/phylink.h>
> #include <linux/rtnetlink.h>
> #include <linux/spinlock.h>
> +#include <linux/timer.h>
> #include <linux/workqueue.h>
>
> #include "sfp.h"
> @@ -54,6 +55,7 @@ struct phylink {
> /* The link configuration settings */
> struct phylink_link_state link_config;
> struct gpio_desc *link_gpio;
> + struct timer_list link_poll;
> void (*get_fixed_state)(struct net_device *dev,
> struct phylink_link_state *s);
>
> @@ -500,6 +502,15 @@ static void phylink_run_resolve(struct phylink *pl)
> queue_work(system_power_efficient_wq, &pl->resolve);
> }
>
> +static void phylink_fixed_poll(struct timer_list *t)
> +{
> + struct phylink *pl = container_of(t, struct phylink, link_poll);
> +
> + mod_timer(t, jiffies + HZ);
> +
> + phylink_run_resolve(pl);
> +}
> +
> static const struct sfp_upstream_ops sfp_phylink_ops;
>
> static int phylink_register_sfp(struct phylink *pl,
> @@ -572,6 +583,7 @@ struct phylink *phylink_create(struct net_device *ndev,
> pl->link_config.an_enabled = true;
> pl->ops = ops;
> __set_bit(PHYLINK_DISABLE_STOPPED, &pl->phylink_disable_state);
> + timer_setup(&pl->link_poll, phylink_fixed_poll, 0);
>
> bitmap_fill(pl->supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
> linkmode_copy(pl->link_config.advertising, pl->supported);
> @@ -905,6 +917,8 @@ void phylink_start(struct phylink *pl)
> clear_bit(PHYLINK_DISABLE_STOPPED, &pl->phylink_disable_state);
> phylink_run_resolve(pl);
>
> + if (pl->link_an_mode == MLO_AN_FIXED && !IS_ERR(pl->link_gpio))
> + mod_timer(&pl->link_poll, jiffies + HZ);
> if (pl->sfp_bus)
> sfp_upstream_start(pl->sfp_bus);
> if (pl->phydev)
> @@ -929,6 +943,8 @@ void phylink_stop(struct phylink *pl)
> phy_stop(pl->phydev);
> if (pl->sfp_bus)
> sfp_upstream_stop(pl->sfp_bus);
> + if (pl->link_an_mode == MLO_AN_FIXED && !IS_ERR(pl->link_gpio))
> + del_timer_sync(&pl->link_poll);
>
> set_bit(PHYLINK_DISABLE_STOPPED, &pl->phylink_disable_state);
> queue_work(system_power_efficient_wq, &pl->resolve);
> --
> 2.14.1
>
--
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 3/9] net: phy: phylink: Poll link GPIOs
From: Florian Fainelli @ 2018-05-10 20:32 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: netdev, privat, andrew, vivien.didelot, davem, sean.wang,
Woojung.Huh, john, cphealy
In-Reply-To: <20180510202900.GZ16141@n2100.armlinux.org.uk>
On 05/10/2018 01:29 PM, Russell King - ARM Linux wrote:
> On Thu, May 10, 2018 at 01:17:31PM -0700, Florian Fainelli wrote:
>> From: Russell King <rmk+kernel@armlinux.org.uk>
>>
>> When using a fixed link with a link GPIO, we need to poll that GPIO to
>> determine link state changes. This is consistent with what fixed_phy.c does.
>>
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>
> I'd like this to use the GPIO interrupt where available, only falling back
> to the timer approach when there's no interrupt. Unfortunately, I don't
> have much time to devote to this at the moment, having recently been away
> on vacation, and now having to work on ARM specific issues for probably
> all of the remainder of this kernel cycle.
>
> That means I won't have time to test your series on any of the boards
> I have available to me.
No worries, thanks for looking at this. Andrew and I both tested this on
the devel B and C boards where this is primarily useful. Can I still get
your SoB for the portions you authored?
I will follow up with a change that uses GPIO interrupts when they are
available.
>
>> ---
>> drivers/net/phy/phylink.c | 16 ++++++++++++++++
>> 1 file changed, 16 insertions(+)
>>
>> diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
>> index 6392b5248cf5..581ce93ecaf9 100644
>> --- a/drivers/net/phy/phylink.c
>> +++ b/drivers/net/phy/phylink.c
>> @@ -19,6 +19,7 @@
>> #include <linux/phylink.h>
>> #include <linux/rtnetlink.h>
>> #include <linux/spinlock.h>
>> +#include <linux/timer.h>
>> #include <linux/workqueue.h>
>>
>> #include "sfp.h"
>> @@ -54,6 +55,7 @@ struct phylink {
>> /* The link configuration settings */
>> struct phylink_link_state link_config;
>> struct gpio_desc *link_gpio;
>> + struct timer_list link_poll;
>> void (*get_fixed_state)(struct net_device *dev,
>> struct phylink_link_state *s);
>>
>> @@ -500,6 +502,15 @@ static void phylink_run_resolve(struct phylink *pl)
>> queue_work(system_power_efficient_wq, &pl->resolve);
>> }
>>
>> +static void phylink_fixed_poll(struct timer_list *t)
>> +{
>> + struct phylink *pl = container_of(t, struct phylink, link_poll);
>> +
>> + mod_timer(t, jiffies + HZ);
>> +
>> + phylink_run_resolve(pl);
>> +}
>> +
>> static const struct sfp_upstream_ops sfp_phylink_ops;
>>
>> static int phylink_register_sfp(struct phylink *pl,
>> @@ -572,6 +583,7 @@ struct phylink *phylink_create(struct net_device *ndev,
>> pl->link_config.an_enabled = true;
>> pl->ops = ops;
>> __set_bit(PHYLINK_DISABLE_STOPPED, &pl->phylink_disable_state);
>> + timer_setup(&pl->link_poll, phylink_fixed_poll, 0);
>>
>> bitmap_fill(pl->supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
>> linkmode_copy(pl->link_config.advertising, pl->supported);
>> @@ -905,6 +917,8 @@ void phylink_start(struct phylink *pl)
>> clear_bit(PHYLINK_DISABLE_STOPPED, &pl->phylink_disable_state);
>> phylink_run_resolve(pl);
>>
>> + if (pl->link_an_mode == MLO_AN_FIXED && !IS_ERR(pl->link_gpio))
>> + mod_timer(&pl->link_poll, jiffies + HZ);
>> if (pl->sfp_bus)
>> sfp_upstream_start(pl->sfp_bus);
>> if (pl->phydev)
>> @@ -929,6 +943,8 @@ void phylink_stop(struct phylink *pl)
>> phy_stop(pl->phydev);
>> if (pl->sfp_bus)
>> sfp_upstream_stop(pl->sfp_bus);
>> + if (pl->link_an_mode == MLO_AN_FIXED && !IS_ERR(pl->link_gpio))
>> + del_timer_sync(&pl->link_poll);
>>
>> set_bit(PHYLINK_DISABLE_STOPPED, &pl->phylink_disable_state);
>> queue_work(system_power_efficient_wq, &pl->resolve);
>> --
>> 2.14.1
>>
>
--
Florian
^ permalink raw reply
* Re: [PATCH v6 5/5] PCI: Remove unused pcie_get_minimum_link()
From: Jeff Kirsher @ 2018-05-10 20:34 UTC (permalink / raw)
To: Bjorn Helgaas, Ganesh Goudar, Michael Chan, Ariel Elior
Cc: linux-pci, everest-linux-l2, intel-wired-lan, netdev,
linux-kernel, Tal Gilboa, Tariq Toukan, Jacob Keller,
Jakub Kicinski
In-Reply-To: <20180510163357.GB190385@bhelgaas-glaptop.roam.corp.google.com>
[-- Attachment #1: Type: text/plain, Size: 1632 bytes --]
On Thu, 2018-05-10 at 11:33 -0500, Bjorn Helgaas wrote:
> On Thu, May 03, 2018 at 03:00:43PM -0500, Bjorn Helgaas wrote:
> > From: Bjorn Helgaas <bhelgaas@google.com>
> >
> > In some cases pcie_get_minimum_link() returned misleading
> > information
> > because it found the slowest link and the narrowest link without
> > considering the total bandwidth of the link.
> >
> > For example, consider a path with these two links:
> >
> > - 16.0 GT/s x1 link (16.0 * 10^9 * 128 / 130) * 1 / 8 = 1969
> > MB/s
> > - 2.5 GT/s x16 link ( 2.5 * 10^9 * 8 / 10) * 16 / 8 = 4000
> > MB/s
> >
> > The available bandwidth of the path is limited by the 16 GT/s link
> > to about
> > 1969 MB/s, but pcie_get_minimum_link() returned 2.5 GT/s x1, which
> > corresponds to only 250 MB/s.
> >
> > Callers should use pcie_print_link_status() instead, or
> > pcie_bandwidth_available() if they need more detailed information.
> >
> > Remove pcie_get_minimum_link() since there are no callers left.
> >
> > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
>
> Hi Jeff,
>
> I got your note that you applied this to dev-queue. I assume that
> means you also applied the preceding patches that removed all the
> users. I got a note about ixgbe, but not the others, so I'm just
> double-checking.
I did initially apply it, but realized that I would have to apply the
earlier patches as well, which did not pertain to the Intel wired LAN
drivers. So I have removed this patch from queue and will only be
testing the ixgbe patch of the series, which Andrew has already tested
and responded to.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v6 4/5] ixgbe: Report PCIe link properties with pcie_print_link_status()
From: Jeff Kirsher @ 2018-05-10 20:37 UTC (permalink / raw)
To: Bjorn Helgaas, Ganesh Goudar, Michael Chan, Ariel Elior
Cc: linux-pci, everest-linux-l2, intel-wired-lan, netdev,
linux-kernel, Tal Gilboa, Tariq Toukan, Jacob Keller,
Jakub Kicinski
In-Reply-To: <152537763602.62474.15659483976051204438.stgit@bhelgaas-glaptop.roam.corp.google.com>
[-- Attachment #1: Type: text/plain, Size: 2368 bytes --]
On Thu, 2018-05-03 at 15:00 -0500, Bjorn Helgaas wrote:
> From: Bjorn Helgaas <bhelgaas@google.com>
>
> Previously the driver used pcie_get_minimum_link() to warn when the
> NIC
> is in a slot that can't supply as much bandwidth as the NIC could
> use.
>
> pcie_get_minimum_link() can be misleading because it finds the
> slowest link
> and the narrowest link (which may be different links) without
> considering
> the total bandwidth of each link. For a path with a 16 GT/s x1 link
> and a
> 2.5 GT/s x16 link, it returns 2.5 GT/s x1, which corresponds to 250
> MB/s of
> bandwidth, not the true available bandwidth of about 1969 MB/s for a
> 16 GT/s x1 link.
>
> Use pcie_print_link_status() to report PCIe link speed and possible
> limitations instead of implementing this in the driver itself. This
> finds
> the slowest link in the path to the device by computing the total
> bandwidth
> of each link and compares that with the capabilities of the device.
>
> The dmesg change is:
>
> - PCI Express bandwidth of %dGT/s available
> - (Speed:%s, Width: x%d, Encoding Loss:%s)
> + %u.%03u Gb/s available PCIe bandwidth (%s x%d link)
>
> or, if the device is capable of better performance than is available
> in the
> current slot:
>
> - This is not sufficient for optimal performance of this card.
> - For optimal performance, at least %dGT/s of bandwidth is
> required.
> - A slot with more lanes and/or higher speed is suggested.
> + %u.%03u Gb/s available PCIe bandwidth, limited by %s x%d link at
> %s (capable of %u.%03u Gb/s with %s x%d link)
>
> Note that the driver previously used dev_warn() to suggest using a
> different slot, but pcie_print_link_status() uses dev_info() because
> if the
> platform has no faster slot available, the user can't do anything
> about the
> warning and may not want to be bothered with it.
>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Acked-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Since this is apart of a series, I am not planning to pick this up and
push to David Miller in my ixgbe updates. This should remain in the
series so David can pick up the entire series at once.
> ---
> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 47 +------------
> ------------
> 1 file changed, 1 insertion(+), 46 deletions(-)
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH net-next] tcp: switch pacing timer to softirq based hrtimer
From: Eric Dumazet @ 2018-05-10 20:55 UTC (permalink / raw)
To: Eric Dumazet, David S . Miller; +Cc: netdev
In-Reply-To: <20180510194909.216521-1-edumazet@google.com>
On 05/10/2018 12:49 PM, Eric Dumazet wrote:
> linux-4.16 got support for softirq based hrtimers.
> TCP can switch its pacing hrtimer to this variant, since this
> avoids going through a tasklet and some atomic operations.
>
I need to send a V2, adding a test of hrtimer_cancel() return value
in tcp_clear_xmit_timers() to eventually release the socket reference.
^ permalink raw reply
* Re: [PATCH net] sctp: remove sctp_chunk_put from fail_mark err path in sctp_ulpevent_make_rcvmsg
From: Marcelo Ricardo Leitner @ 2018-05-10 14:10 UTC (permalink / raw)
To: Xin Long; +Cc: network dev, linux-sctp, davem, Neil Horman, syzkaller
In-Reply-To: <e4d7cf118c028ed05c9005951e30babc8bb300eb.1525944853.git.lucien.xin@gmail.com>
On Thu, May 10, 2018 at 05:34:13PM +0800, Xin Long wrote:
> In Commit 1f45f78f8e51 ("sctp: allow GSO frags to access the chunk too"),
> it held the chunk in sctp_ulpevent_make_rcvmsg to access it safely later
> in recvmsg. However, it also added sctp_chunk_put in fail_mark err path,
> which is only triggered before holding the chunk.
>
> syzbot reported a use-after-free crash happened on this err path, where
> it shouldn't call sctp_chunk_put.
>
> This patch simply removes this call.
>
> Fixes: 1f45f78f8e51 ("sctp: allow GSO frags to access the chunk too")
> Reported-by: syzbot+141d898c5f24489db4aa@syzkaller.appspotmail.com
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> ---
> net/sctp/ulpevent.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/net/sctp/ulpevent.c b/net/sctp/ulpevent.c
> index 84207ad..8cb7d98 100644
> --- a/net/sctp/ulpevent.c
> +++ b/net/sctp/ulpevent.c
> @@ -715,7 +715,6 @@ struct sctp_ulpevent *sctp_ulpevent_make_rcvmsg(struct sctp_association *asoc,
> return event;
>
> fail_mark:
> - sctp_chunk_put(chunk);
> kfree_skb(skb);
> fail:
> return NULL;
> --
> 2.1.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* linux-next: Signed-off-by missing for commit in the net tree
From: Stephen Rothwell @ 2018-05-10 21:17 UTC (permalink / raw)
To: David Miller, Networking
Cc: Linux-Next Mailing List, Linux Kernel Mailing List, Hangbin Liu
[-- Attachment #1: Type: text/plain, Size: 176 bytes --]
Hi all,
Commit
0e8411e426e2 ("ipv4: reset fnhe_mtu_locked after cache route flushed")
is missing a Signed-off-by from its author.
--
Cheers,
Stephen Rothwell
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH ghak81 RFC V1 3/5] audit: use inline function to get audit context
From: Richard Guy Briggs @ 2018-05-10 21:17 UTC (permalink / raw)
To: Paul Moore
Cc: Linux-Audit Mailing List, LKML,
Linux NetDev Upstream Mailing List, Netfilter Devel List,
Linux Security Module list, Integrity Measurement Architecture,
SElinux list, Eric Paris, Steve Grubb, Ingo Molnar, David Howells
In-Reply-To: <CAHC9VhSbMaJ72jJnEivDd-M2UXDomexva8B-xsWXapTyeF0JVQ@mail.gmail.com>
On 2018-05-09 11:28, Paul Moore wrote:
> On Fri, May 4, 2018 at 4:54 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
> > Recognizing that the audit context is an internal audit value, use an
> > access function to retrieve the audit context pointer for the task
> > rather than reaching directly into the task struct to get it.
> >
> > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > ---
> > include/linux/audit.h | 16 ++++++++---
> > include/net/xfrm.h | 2 +-
> > kernel/audit.c | 4 +--
> > kernel/audit_watch.c | 2 +-
> > kernel/auditsc.c | 52 ++++++++++++++++++------------------
> > net/bridge/netfilter/ebtables.c | 2 +-
> > net/core/dev.c | 2 +-
> > net/netfilter/x_tables.c | 2 +-
> > net/netlabel/netlabel_user.c | 2 +-
> > security/integrity/ima/ima_api.c | 2 +-
> > security/integrity/integrity_audit.c | 2 +-
> > security/lsm_audit.c | 2 +-
> > security/selinux/hooks.c | 4 +--
> > security/selinux/selinuxfs.c | 6 ++---
> > security/selinux/ss/services.c | 12 ++++-----
> > 15 files changed, 60 insertions(+), 52 deletions(-)
> >
> > diff --git a/include/linux/audit.h b/include/linux/audit.h
> > index 5f86f7c..93e4c61 100644
> > --- a/include/linux/audit.h
> > +++ b/include/linux/audit.h
> > @@ -235,26 +235,30 @@ extern void __audit_inode_child(struct inode *parent,
> > extern void __audit_seccomp(unsigned long syscall, long signr, int code);
> > extern void __audit_ptrace(struct task_struct *t);
> >
> > +static inline struct audit_context *audit_context(struct task_struct *task)
> > +{
> > + return task->audit_context;
> > +}
>
> Another case where I think I agree with everything here on principle,
> especially when one considers it in the larger context of the audit
> container ID work. However, I think we might be able to somply this a
> bit by eliminating the parameter to the new audit_context() helper and
> making it always reference the current task_struct. Based on this
> patch it would appear that this change would work for all callers
> except for audit_take_context() and __audit_syscall_entry(), both of
> which are contained within the core audit code and are enough of a
> special case that I think it is acceptable for them to access the
> context directly. I'm trying to think of reasons why a non-audit
> kernel subsystem would ever need to access the audit context of a
> process other than current and I can't think of any ... removing the
> task_struct pointer might help prevent mistakes/abuse in the future.
As for __audit_syscall_{entry,exit}() and audit_signal_info(), they are
using current. current is assigned to local variable tsk only to be
used as the LHS in assignments and for locking.
But, audit_take_context() and audit_log_exit() are both called also from
__audit_free() which can have non-current handed to it by copy_process()
cleaning up, while do_exit() appears to still be in current.
So, Ok, ditch the parameter to audit_context() and use local access when
needed.
> > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> > index 6e3ceb9..a4bbdcc 100644
> > --- a/kernel/auditsc.c
> > +++ b/kernel/auditsc.c
> > @@ -836,7 +836,7 @@ static inline struct audit_context *audit_take_context(struct task_struct *tsk,
> > int return_valid,
> > long return_code)
> > {
> > - struct audit_context *context = tsk->audit_context;
> > + struct audit_context *context = audit_context(tsk);
> >
> > if (!context)
> > return NULL;
> > @@ -1510,7 +1510,7 @@ void __audit_syscall_entry(int major, unsigned long a1, unsigned long a2,
> > unsigned long a3, unsigned long a4)
> > {
> > struct task_struct *tsk = current;
> > - struct audit_context *context = tsk->audit_context;
> > + struct audit_context *context = audit_context(tsk);
> > enum audit_state state;
> >
> > if (!audit_enabled || !context)
>
> --
> paul moore
> www.paul-moore.com
- RGB
--
Richard Guy Briggs <rgb@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635
^ permalink raw reply
* Re: [PATCH ghak81 RFC V1 1/5] audit: normalize loginuid read access
From: Richard Guy Briggs @ 2018-05-10 21:21 UTC (permalink / raw)
To: Paul Moore
Cc: Linux-Audit Mailing List, LKML,
Linux NetDev Upstream Mailing List, Netfilter Devel List,
Linux Security Module list, Integrity Measurement Architecture,
SElinux list, Eric Paris, Steve Grubb, Ingo Molnar, David Howells
In-Reply-To: <CAHC9VhQKYt0PC0L65pwFRte1D98R=2tUDGxMVpc8bbJsMncGpw@mail.gmail.com>
On 2018-05-09 11:13, Paul Moore wrote:
> On Fri, May 4, 2018 at 4:54 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
> > Recognizing that the loginuid is an internal audit value, use an access
> > function to retrieve the audit loginuid value for the task rather than
> > reaching directly into the task struct to get it.
> >
> > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > ---
> > kernel/auditsc.c | 16 ++++++++--------
> > 1 file changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> > index 479c031..f3817d0 100644
> > --- a/kernel/auditsc.c
> > +++ b/kernel/auditsc.c
> > @@ -374,7 +374,7 @@ static int audit_field_compare(struct task_struct *tsk,
> > case AUDIT_COMPARE_EGID_TO_OBJ_GID:
> > return audit_compare_gid(cred->egid, name, f, ctx);
> > case AUDIT_COMPARE_AUID_TO_OBJ_UID:
> > - return audit_compare_uid(tsk->loginuid, name, f, ctx);
> > + return audit_compare_uid(audit_get_loginuid(tsk), name, f, ctx);
> > case AUDIT_COMPARE_SUID_TO_OBJ_UID:
> > return audit_compare_uid(cred->suid, name, f, ctx);
> > case AUDIT_COMPARE_SGID_TO_OBJ_GID:
> > @@ -385,7 +385,7 @@ static int audit_field_compare(struct task_struct *tsk,
> > return audit_compare_gid(cred->fsgid, name, f, ctx);
> > /* uid comparisons */
> > case AUDIT_COMPARE_UID_TO_AUID:
> > - return audit_uid_comparator(cred->uid, f->op, tsk->loginuid);
> > + return audit_uid_comparator(cred->uid, f->op, audit_get_loginuid(tsk));
> > case AUDIT_COMPARE_UID_TO_EUID:
> > return audit_uid_comparator(cred->uid, f->op, cred->euid);
> > case AUDIT_COMPARE_UID_TO_SUID:
> > @@ -394,11 +394,11 @@ static int audit_field_compare(struct task_struct *tsk,
> > return audit_uid_comparator(cred->uid, f->op, cred->fsuid);
> > /* auid comparisons */
> > case AUDIT_COMPARE_AUID_TO_EUID:
> > - return audit_uid_comparator(tsk->loginuid, f->op, cred->euid);
> > + return audit_uid_comparator(audit_get_loginuid(tsk), f->op, cred->euid);
> > case AUDIT_COMPARE_AUID_TO_SUID:
> > - return audit_uid_comparator(tsk->loginuid, f->op, cred->suid);
> > + return audit_uid_comparator(audit_get_loginuid(tsk), f->op, cred->suid);
> > case AUDIT_COMPARE_AUID_TO_FSUID:
> > - return audit_uid_comparator(tsk->loginuid, f->op, cred->fsuid);
> > + return audit_uid_comparator(audit_get_loginuid(tsk), f->op, cred->fsuid);
> > /* euid comparisons */
> > case AUDIT_COMPARE_EUID_TO_SUID:
> > return audit_uid_comparator(cred->euid, f->op, cred->suid);
> > @@ -611,7 +611,7 @@ static int audit_filter_rules(struct task_struct *tsk,
> > result = match_tree_refs(ctx, rule->tree);
> > break;
> > case AUDIT_LOGINUID:
> > - result = audit_uid_comparator(tsk->loginuid, f->op, f->uid);
> > + result = audit_uid_comparator(audit_get_loginuid(tsk), f->op, f->uid);
> > break;
> > case AUDIT_LOGINUID_SET:
> > result = audit_comparator(audit_loginuid_set(tsk), f->op, f->val);
> > @@ -2287,8 +2287,8 @@ int audit_signal_info(int sig, struct task_struct *t)
> > (sig == SIGTERM || sig == SIGHUP ||
> > sig == SIGUSR1 || sig == SIGUSR2)) {
> > audit_sig_pid = task_tgid_nr(tsk);
> > - if (uid_valid(tsk->loginuid))
> > - audit_sig_uid = tsk->loginuid;
> > + if (uid_valid(audit_get_loginuid(tsk)))
> > + audit_sig_uid = audit_get_loginuid(tsk);
>
> I realize this comment is a little silly given the nature of loginuid,
> but if we are going to abstract away loginuid accesses (which I think
> is good), we should probably access it once, store it in a local
> variable, perform the validity check on the local variable, then
> commit the local variable to audit_sig_uid. I realize a TOCTOU
> problem is unlikely here, but with this new layer of abstraction it
> seems that some additional safety might be a good thing.
Ok, I'll just assign it to where it is going and check it there, holding
the audit_ctl_lock the whole time, since it should have been done
anyways for all of audit_sig_{pid,uid,sid} anyways to get a consistent
view from the AUDIT_SIGNAL_INFO fetch.
> > else
> > audit_sig_uid = uid;
> > security_task_getsecid(tsk, &audit_sig_sid);
> paul moore
- RGB
--
Richard Guy Briggs <rgb@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635
^ permalink raw reply
* Re: [PATCH v3 next-next] drivers: net: davinci_mdio: prevent spurious timeout
From: David Miller @ 2018-05-10 21:23 UTC (permalink / raw)
To: nsekhar; +Cc: grygorii.strashko, linux-omap, netdev, andrew
In-Reply-To: <20180509154515.5968-1-nsekhar@ti.com>
From: Sekhar Nori <nsekhar@ti.com>
Date: Wed, 9 May 2018 21:15:15 +0530
> A well timed kernel preemption in the time_after() loop
> in wait_for_idle() can result in a spurious timeout
> error to be returned.
>
> Fix it by using readl_poll_timeout() which takes care of
> this issue.
>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Sekhar Nori <nsekhar@ti.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] hv_netvsc: typo in NDIS RSS parameters structure
From: David Miller @ 2018-05-10 21:23 UTC (permalink / raw)
To: stephen; +Cc: netdev, sthemmin
In-Reply-To: <20180509160007.8289-1-sthemmin@microsoft.com>
From: Stephen Hemminger <stephen@networkplumber.org>
Date: Wed, 9 May 2018 09:00:07 -0700
> Fix simple misspelling kashkey_offset should be hashkey_offset.
>
> Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
Applied.
^ permalink raw reply
* Re: [PATCH ghak81 RFC V1 5/5] audit: collect audit task parameters
From: Richard Guy Briggs @ 2018-05-10 21:26 UTC (permalink / raw)
To: Paul Moore
Cc: Linux NetDev Upstream Mailing List, LKML, David Howells,
Linux Security Module list, Linux-Audit Mailing List,
Netfilter Devel List, SElinux list,
Integrity Measurement Architecture, Ingo Molnar
In-Reply-To: <CAHC9VhRugVt3g=ADwKKWLYa2NXVoL8HLRKtXsut3P2LPd3fPuw@mail.gmail.com>
On 2018-05-09 11:46, Paul Moore wrote:
> On Fri, May 4, 2018 at 4:54 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
> > The audit-related parameters in struct task_struct should ideally be
> > collected together and accessed through a standard audit API.
> >
> > Collect the existing loginuid, sessionid and audit_context together in a
> > new struct audit_task_info pointer called "audit" in struct task_struct.
> >
> > Use kmem_cache to manage this pool of memory.
> > Un-inline audit_free() to be able to always recover that memory.
> >
> > See: https://github.com/linux-audit/audit-kernel/issues/81
> >
> > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > ---
> > MAINTAINERS | 2 +-
> > include/linux/audit.h | 8 ++++----
> > include/linux/audit_task.h | 31 +++++++++++++++++++++++++++++++
> > include/linux/sched.h | 6 ++----
> > init/init_task.c | 8 ++++++--
> > kernel/auditsc.c | 4 ++--
> > 6 files changed, 46 insertions(+), 13 deletions(-)
> > create mode 100644 include/linux/audit_task.h
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 0a1410d..8c7992d 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -2510,7 +2510,7 @@ L: linux-audit@redhat.com (moderated for non-subscribers)
> > W: https://github.com/linux-audit
> > T: git git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/audit.git
> > S: Supported
> > -F: include/linux/audit.h
> > +F: include/linux/audit*.h
> > F: include/uapi/linux/audit.h
> > F: kernel/audit*
> >
> > diff --git a/include/linux/audit.h b/include/linux/audit.h
> > index dba0d45..1324969 100644
> > --- a/include/linux/audit.h
> > +++ b/include/linux/audit.h
> > @@ -237,11 +237,11 @@ extern void __audit_inode_child(struct inode *parent,
> >
> > static inline void audit_set_context(struct task_struct *task, struct audit_context *ctx)
> > {
> > - task->audit_context = ctx;
> > + task->audit.ctx = ctx;
> > }
> > static inline struct audit_context *audit_context(struct task_struct *task)
> > {
> > - return task->audit_context;
> > + return task->audit.ctx;
> > }
> > static inline bool audit_dummy_context(void)
> > {
> > @@ -330,12 +330,12 @@ extern int auditsc_get_stamp(struct audit_context *ctx,
> >
> > static inline kuid_t audit_get_loginuid(struct task_struct *tsk)
> > {
> > - return tsk->loginuid;
> > + return tsk->audit.loginuid;
> > }
> >
> > static inline unsigned int audit_get_sessionid(struct task_struct *tsk)
> > {
> > - return tsk->sessionid;
> > + return tsk->audit.sessionid;
> > }
> >
> > extern void __audit_ipc_obj(struct kern_ipc_perm *ipcp);
> > diff --git a/include/linux/audit_task.h b/include/linux/audit_task.h
> > new file mode 100644
> > index 0000000..d4b3a20
> > --- /dev/null
> > +++ b/include/linux/audit_task.h
> > @@ -0,0 +1,31 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/* audit_task.h -- definition of audit_task_info structure
> > + *
> > + * Copyright 2018 Red Hat Inc., Raleigh, North Carolina.
> > + * All Rights Reserved.
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + *
> > + * Written by Richard Guy Briggs <rgb@redhat.com>
> > + *
> > + */
> > +
> > +#ifndef _LINUX_AUDIT_TASK_H_
> > +#define _LINUX_AUDIT_TASK_H_
> > +
> > +struct audit_context;
> > +struct audit_task_info {
> > + kuid_t loginuid;
> > + unsigned int sessionid;
> > + struct audit_context *ctx;
> > +};
> > +
> > +#endif
> > diff --git a/include/linux/sched.h b/include/linux/sched.h
> > index b3d697f..b58eca0 100644
> > --- a/include/linux/sched.h
> > +++ b/include/linux/sched.h
> > @@ -27,9 +27,9 @@
> > #include <linux/signal_types.h>
> > #include <linux/mm_types_task.h>
> > #include <linux/task_io_accounting.h>
> > +#include <linux/audit_task.h>
> >
> > /* task_struct member predeclarations (sorted alphabetically): */
> > -struct audit_context;
> > struct backing_dev_info;
> > struct bio_list;
> > struct blk_plug;
> > @@ -832,10 +832,8 @@ struct task_struct {
> >
> > struct callback_head *task_works;
> >
> > - struct audit_context *audit_context;
> > #ifdef CONFIG_AUDITSYSCALL
> > - kuid_t loginuid;
> > - unsigned int sessionid;
> > + struct audit_task_info audit;
> > #endif
>
> Considering that the audit_context pointer is now in the
> audit_task_info struct, should the audit_task_info struct be placed
> outside the CONFIG_AUDITSYSCALL protections? Or rather, shouldn't the
> CONFIG_AUDITSYSCALL protections be moved inside audit_task_info or
> removed entirely?
Well, I wondered about that anyways. audit_context is only meaningful
in CONFIG_AUDIT_SYSCALL, and loginuid and sessionid were already there,
so the whole thing should be inside, but given that CONFIG_AUDIT_SYSCALL
is forced on when CONFIG_AUDIT is set I don't see that it matters.
Perhaps CONFIG_AUDIT_SYSCALL should be ripped out completely and the
code flattenned to the CONFIG_AUDIT case.
I see your point though, moving CONFIG_AUDIT_SYSCALL protections to
within the audit_task_info struct definition makes more sense than this
above.
> > diff --git a/init/init_task.c b/init/init_task.c
> > index c788f91..d33260d 100644
> > --- a/init/init_task.c
> > +++ b/init/init_task.c
> > @@ -9,6 +9,7 @@
> > #include <linux/init.h>
> > #include <linux/fs.h>
> > #include <linux/mm.h>
> > +#include <linux/audit.h>
> >
> > #include <asm/pgtable.h>
> > #include <linux/uaccess.h>
> > @@ -118,8 +119,11 @@ struct task_struct init_task
> > .thread_group = LIST_HEAD_INIT(init_task.thread_group),
> > .thread_node = LIST_HEAD_INIT(init_signals.thread_head),
> > #ifdef CONFIG_AUDITSYSCALL
> > - .loginuid = INVALID_UID,
> > - .sessionid = AUDIT_SID_UNSET,
> > + .audit = {
> > + .loginuid = INVALID_UID,
> > + .sessionid = AUDIT_SID_UNSET,
> > + .ctx = NULL,
> > + },
> > #endif
> > #ifdef CONFIG_PERF_EVENTS
> > .perf_event_mutex = __MUTEX_INITIALIZER(init_task.perf_event_mutex),
> > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> > index f294e4a..b5d8bff 100644
> > --- a/kernel/auditsc.c
> > +++ b/kernel/auditsc.c
> > @@ -2068,8 +2068,8 @@ int audit_set_loginuid(kuid_t loginuid)
> > sessionid = (unsigned int)atomic_inc_return(&session_id);
> > }
> >
> > - task->sessionid = sessionid;
> > - task->loginuid = loginuid;
> > + task->audit.sessionid = sessionid;
> > + task->audit.loginuid = loginuid;
> > out:
> > audit_log_set_loginuid(oldloginuid, loginuid, oldsessionid, sessionid, rc);
> > return rc;
> > --
> > 1.8.3.1
>
> --
> paul moore
> www.paul-moore.com
- RGB
--
Richard Guy Briggs <rgb@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635
^ permalink raw reply
* Re: [PATCH net-next 0/4] Misc bug fixes for HNS3 Ethernet Driver
From: David Miller @ 2018-05-10 21:27 UTC (permalink / raw)
To: salil.mehta
Cc: yisen.zhuang, lipeng321, mehta.salil, netdev, linux-kernel,
linuxarm
In-Reply-To: <20180509162441.18068-1-salil.mehta@huawei.com>
From: Salil Mehta <salil.mehta@huawei.com>
Date: Wed, 9 May 2018 17:24:37 +0100
> Fixes to some of the bugs found during system test, internal review
> and clean-up
Series applied, thank you.
^ permalink raw reply
* Re: [PATCH net] tc-testing: fix tdc tests for 'bpf' action
From: David Miller @ 2018-05-10 21:28 UTC (permalink / raw)
To: dcaratti; +Cc: mrv, lucasb, netdev
In-Reply-To: <54d69bac92e9c0a216997261ef7b1e0eb0dd28c9.1525884149.git.dcaratti@redhat.com>
From: Davide Caratti <dcaratti@redhat.com>
Date: Wed, 9 May 2018 18:45:42 +0200
> - correct a typo in the value of 'matchPattern' of test 282d, potentially
> causing false negative
> - allow errors when 'teardown' executes '$TC action flush action bpf' in
> test 282d, to fix false positive when it is run with act_bpf unloaded
> - correct the value of 'matchPattern' in test e939, causing false positive
> in case the BPF JIT is enabled
>
> Fixes: 440ea4ae1828 ("tc-testing: add selftests for 'bpf' action")
> Signed-off-by: Davide Caratti <dcaratti@redhat.com>
Applied.
^ permalink raw reply
* Re: [PATCH net] tipc: fix one byte leak in tipc_sk_set_orig_addr()
From: David Miller @ 2018-05-10 21:29 UTC (permalink / raw)
To: edumazet; +Cc: netdev, eric.dumazet, jon.maloy, ying.xue
In-Reply-To: <20180509165022.199827-1-edumazet@google.com>
From: Eric Dumazet <edumazet@google.com>
Date: Wed, 9 May 2018 09:50:22 -0700
> sysbot/KMSAN reported an uninit-value in recvmsg() that
> I tracked down to tipc_sk_set_orig_addr(), missing
> srcaddr->member.scope initialization.
>
> This patches moves srcaddr->sock.scope init to follow
> fields order and ease future verifications.
...
> Fixes: 31c82a2d9d51 ("tipc: add second source address to recvmsg()/recvfrom()")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: syzbot <syzkaller@googlegroups.com>
Applied and queued up for -stable.
^ permalink raw reply
* Re: [PATCH net] net/ipv6: fix lock imbalance in ip6_route_del()
From: David Miller @ 2018-05-10 21:30 UTC (permalink / raw)
To: edumazet; +Cc: netdev, eric.dumazet, dsahern
In-Reply-To: <20180509170546.247826-1-edumazet@google.com>
From: Eric Dumazet <edumazet@google.com>
Date: Wed, 9 May 2018 10:05:46 -0700
> WARNING: lock held when returning to user space!
> 4.17.0-rc3+ #37 Not tainted
...
> Fixes: 23fb93a4d3f1 ("net/ipv6: Cleanup exception and cache route handling")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: David Ahern <dsahern@gmail.com>
> Reported-by: syzbot <syzkaller@googlegroups.com>
Applied to net-next.
^ permalink raw reply
* Re: [net-next v2 0/6][pull request] 100GbE Intel Wired LAN Driver Updates 2018-05-09
From: David Miller @ 2018-05-10 21:31 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, nhorman, sassmann, jogreene
In-Reply-To: <20180509181011.30907-1-jeffrey.t.kirsher@intel.com>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Wed, 9 May 2018 11:10:05 -0700
> This series contains updates to fm10k only.
>
> Jake provides all the changes in the series, starting with adding
> support for accelerated MACVLAN devices. Reduced code duplication by
> implementing a macro to be used when setting up the type specific
> macros. Avoided potential bugs with stats by using a macro to calculate
> the array size when passing to ensure that the size is correct.
>
> v2: changed macro reference '#' with __stringify() as suggested by
> Joe Perches to patch 2 of the series. Also made sure the updated
> series of patches is actually pushed to my kernel.org tree
...
> git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue 100GbE
Pulled, thanks Jeff.
^ permalink raw reply
* Re: [PATCH net-next] liquidio: monitor all of Octeon's cores in watchdog thread
From: David Miller @ 2018-05-10 21:32 UTC (permalink / raw)
To: felix.manlunas; +Cc: netdev, raghu.vatsavayi, derek.chickles, satananda.burla
In-Reply-To: <20180509183131.GA1811@felix-thinkpad.cavium.com>
From: Felix Manlunas <felix.manlunas@cavium.com>
Date: Wed, 9 May 2018 11:31:31 -0700
> The liquidio_watchdog kernel thread is watching over only 12 cores of the
> Octeon CN23XX; it's neglecting the other 4 cores that are present in the
> CN2360. Fix it by defining LIO_MAX_CORES as 16.
>
> Signed-off-by: Felix Manlunas <felix.manlunas@cavium.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] liquidio: bump up driver version to 1.7.2 to match newer NIC firmware
From: David Miller @ 2018-05-10 21:32 UTC (permalink / raw)
To: felix.manlunas; +Cc: netdev, raghu.vatsavayi, derek.chickles, satananda.burla
In-Reply-To: <20180509184938.GA1852@felix-thinkpad.cavium.com>
From: Felix Manlunas <felix.manlunas@cavium.com>
Date: Wed, 9 May 2018 11:49:38 -0700
> Signed-off-by: Felix Manlunas <felix.manlunas@cavium.com>
Applied.
^ permalink raw reply
* Re: pull-request: mac80211 2018-05-09
From: David Miller @ 2018-05-10 21:35 UTC (permalink / raw)
To: johannes; +Cc: netdev, linux-wireless
In-Reply-To: <20180509193613.10902-1-johannes@sipsolutions.net>
From: Johannes Berg <johannes@sipsolutions.net>
Date: Wed, 9 May 2018 21:36:12 +0200
> We just have a few fixes this time around.
>
> Please pull and let me know if there's any problem.
Pulled, thank you!
^ permalink raw reply
* Re: pull-request: mac80211-next 2018-05-09
From: David Miller @ 2018-05-10 21:35 UTC (permalink / raw)
To: johannes; +Cc: netdev, linux-wireless
In-Reply-To: <1525901377.6910.29.camel@sipsolutions.net>
From: Johannes Berg <johannes@sipsolutions.net>
Date: Wed, 09 May 2018 23:29:37 +0200
> Hi,
>
> Sorry, scratch that.
>
> I forgot that this commit:
>
>> Toke Høiland-Jørgensen (3):
>
>> cfg80211: Expose TXQ stats and parameters to userspace
>
> caused a bunch of "too much stack" warnings - I should put in at least
> the non-driver fix for that first, and then coordinate with Kalle to
> send the driver fixes in too.
Ok, tossed.
^ permalink raw reply
* Re: [PATCH net] hv_netvsc: set master device
From: David Miller @ 2018-05-10 21:36 UTC (permalink / raw)
To: stephen; +Cc: netdev, sthemmin
In-Reply-To: <20180509210904.21406-1-sthemmin@microsoft.com>
From: Stephen Hemminger <stephen@networkplumber.org>
Date: Wed, 9 May 2018 14:09:04 -0700
> The hyper-v transparent bonding should have used master_dev_link.
> The netvsc device should look like a master bond device not
> like the upper side of a tunnel.
>
> This makes the semantics the same so that userspace applications
> looking at network devices see the correct master relationshipship.
>
> Fixes: 0c195567a8f6 ("netvsc: transparent VF management")
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Applied and queued up for -stable.
^ permalink raw reply
* Re: [PATCH net-next] net/core: correct the variable name in dev_ioctl() comment
From: David Miller @ 2018-05-10 21:41 UTC (permalink / raw)
To: sunlw.fnst; +Cc: netdev
In-Reply-To: <20180510030120.4747-1-sunlw.fnst@cn.fujitsu.com>
From: Sun Lianwen <sunlw.fnst@cn.fujitsu.com>
Date: Thu, 10 May 2018 11:01:20 +0800
> The variable name is not "arg" but "ifr" in dev_ioctl()
>
> Signed-off-by: Sun Lianwen <sunlw.fnst@cn.fujitsu.com>
If you are going to touch this, fix it full by adding the need_copyout
variable to the comment as well.
^ permalink raw reply
* Re: [PATCH] net: ipv4: remove define INET_CSK_DEBUG and unnecessary EXPORT_SYMBOL
From: David Miller @ 2018-05-10 21:44 UTC (permalink / raw)
To: joe; +Cc: kuznet, yoshfuji, lirongqing, acme, netdev, linux-kernel
In-Reply-To: <0424e034b4640359bbe1ae50229b9fbc25b06181.1525932412.git.joe@perches.com>
From: Joe Perches <joe@perches.com>
Date: Wed, 9 May 2018 23:24:07 -0700
> INET_CSK_DEBUG is always set and only is used for 2 pr_debug calls.
>
> EXPORT_SYMBOL(inet_csk_timer_bug_msg) is only used by these 2
> pr_debug calls and is also unnecessary as the exported string can
> be used directly by these calls.
>
> Signed-off-by: Joe Perches <joe@perches.com>
Applied to net-next.
^ permalink raw reply
* Re: [PATCH net-next v2] tcp: Add mark for TIMEWAIT sockets
From: David Miller @ 2018-05-10 21:45 UTC (permalink / raw)
To: jmaxwell37; +Cc: kuznet, yoshfuji, netdev, linux-kernel, jmaxwell
In-Reply-To: <20180510065351.22535-1-jmaxwell37@gmail.com>
From: Jon Maxwell <jmaxwell37@gmail.com>
Date: Thu, 10 May 2018 16:53:51 +1000
> This version has some suggestions by Eric Dumazet:
>
> - Use a local variable for the mark in IPv6 instead of ctl_sk to avoid SMP
> races.
> - Use the more elegant "IP4_REPLY_MARK(net, skb->mark) ?: sk->sk_mark"
> statement.
> - Factorize code as sk_fullsock() check is not necessary.
>
> Aidan McGurn from Openwave Mobility systems reported the following bug:
>
> "Marked routing is broken on customer deployment. Its effects are large
> increase in Uplink retransmissions caused by the client never receiving
> the final ACK to their FINACK - this ACK misses the mark and routes out
> of the incorrect route."
>
> Currently marks are added to sk_buffs for replies when the "fwmark_reflect"
> sysctl is enabled. But not for TW sockets that had sk->sk_mark set via
> setsockopt(SO_MARK..).
>
> Fix this in IPv4/v6 by adding tw->tw_mark for TIME_WAIT sockets. Copy the the
> original sk->sk_mark in __inet_twsk_hashdance() to the new tw->tw_mark location.
> Then progate this so that the skb gets sent with the correct mark. Do the same
> for resets. Give the "fwmark_reflect" sysctl precedence over sk->sk_mark so that
> netfilter rules are still honored.
>
> Signed-off-by: Jon Maxwell <jmaxwell37@gmail.com>
I'm surprised the lack of a mark in timewait sockets wasn't noticed earlier.
Applied, thank you.
^ 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