On Thu, 2026-02-05 at 14:42 +0100, Antonio Quartulli wrote: > On 05/02/2026 11:42, Sabrina Dubroca wrote: > > 2026-02-04, 23:42:31 +0100, Antonio Quartulli wrote: > > > > > @@ -113,7 +132,7 @@ static void ovpn_tcp_rcv(struct strparser > > > > > *strp, struct sk_buff *skb) > > > > > /* The packet size header must be there when > > > > > sending the packet > > > > > * to userspace, therefore we put it back > > > > > */ > > > > > - skb_push(skb, 2); > > > > > + *((__force __be16 *)__skb_push(skb, 2)) = > > > > > cpu_to_be16(pkt_len); > > > > > > > > Why switch to __skb_push here? The skb_under_panic() would be > > > > nice in > > > > the (unexpected) case we have an invalid skb. > > > > > > > > > > We just created the skb, so it should be safe to assume that the > > > skb is > > > valid, no? > > > > I meant invalid in the sense of "not having the expected room to > > push > > the header". > > > > > > Here we rely on the extra room that netdev_alloc_skb reserves > > > > (NET_SKB_PAD) in case we have to re-push the packet length, > > > > right? > > > > > > The comment at > > > https://elixir.bootlin.com/linux/v6.19-rc5/source/include/linux/skbuff.h#L3282 > > > about NET_SKB_PAD seems to suggest that this is actual headroom > > > that > > > networking drivers can assume to have, no? > > > > I guess so. But I'd feel a bit safer if we had something enforcing > > that at least this push can't fail, more as a guarantee against an > > accidental code change in ovpn. But maybe we'd have bigger problems > > through the rest of the stack if ovpn skbs get bridged/forwarded and > > we don't have all that headroom. > > Exactly my thought. If this assumption doesn't hold anymore, more > things > are going to break. > (I expect also other mechanisms outside ovpn to be affected) > > On top of that, we ovpn_tcp_send_skb() basically relying on the same > assumption already. > Therefore, I'd just stick with the current version. > > However, I have a little nitpick: Ralf, please use the same style as > the > assignment in ovpn_tcp_send_skb(), for consistency: > > 584 *(__be16 *)__skb_push(skb, sizeof(u16)) = htons(len);
ACK. > Regards, > > > > > > And it should never be less than 32 bytes. > > > > > > > > > Regards, > > -- Ralf Lici Mandelbit Srl _______________________________________________ Openvpn-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/openvpn-devel
