lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Mon, 20 Sep 2021 01:44:29 -0400 From: Tyler Stachecki <stachecki.tyler@...il.com> To: Cong Wang <xiyou.wangcong@...il.com> Cc: fankaixi.li@...edance.com, xiexiaohui.xxh@...edance.com, "Cong Wang ." <cong.wang@...edance.com>, Pravin B Shelar <pshelar@....org>, "David S. Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, Linux Kernel Network Developers <netdev@...r.kernel.org>, dev@...nvswitch.org, LKML <linux-kernel@...r.kernel.org> Subject: Re: [PATCH] ovs: Only clear tstamp when changing namespaces On Sun, Sep 19, 2021 at 7:33 PM Cong Wang <xiyou.wangcong@...il.com> wrote: > > On Sun, Sep 19, 2021 at 10:59 AM Tyler J. Stachecki > <stachecki.tyler@...il.com> wrote: > > > > As of "ovs: clear skb->tstamp in forwarding path", the > > tstamp is now being cleared unconditionally to fix fq qdisc > > operation with ovs vports. > > > > While this is mostly correct and fixes forwarding for that > > use case, a slight adjustment is necessary to ensure that > > the tstamp is cleared *only when the forwarding is across > > namespaces*. > > Hmm? I am sure timestamp has already been cleared when > crossing netns: > > void skb_scrub_packet(struct sk_buff *skb, bool xnet) > { > ... > if (!xnet) > return; > > ipvs_reset(skb); > skb->mark = 0; > skb->tstamp = 0; > } > > So, what are you trying to fix? > > > > > Signed-off-by: Tyler J. Stachecki <stachecki.tyler@...il.com> > > --- > > net/openvswitch/vport.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/net/openvswitch/vport.c b/net/openvswitch/vport.c > > index cf2ce5812489..c2d32a5c3697 100644 > > --- a/net/openvswitch/vport.c > > +++ b/net/openvswitch/vport.c > > @@ -507,7 +507,8 @@ void ovs_vport_send(struct vport *vport, struct sk_buff *skb, u8 mac_proto) > > } > > > > skb->dev = vport->dev; > > - skb->tstamp = 0; > > + if (dev_net(skb->dev)) > > Doesn't dev_net() always return a non-NULL pointer? > > If you really want to check whether it is cross-netns, you should > use net_eq() to compare src netns with dst netns, something like: > if (!net_eq(dev_net(vport->dev), dev_net(skb->dev))). > > Thanks. Sorry if this is a no-op -- I'm admittedly not familiar with this part of the tree. I had added this check based on this discussion on the OVS mailing list: https://mail.openvswitch.org/pipermail/ovs-discuss/2021-February/050966.html The motivation to add it was based on the fact that skb_scrub_packet is doing it conditionally as well, but you seem to indicate that skb_scrub_packet itself is already being done somewhere? Cheers, Tyler
Powered by blists - more mailing lists