On Wed, 9 Feb 2022, Jakub Kicinski wrote:
> On Mon, 7 Feb 2022 16:41:01 +0200 Paul Blakey wrote:
> > Ipv6 ttl, label and tos fields are modified without first
> > pulling/pushing the ipv6 header, which would have updated
> > the hw csum (if available). This might cause csum validation
> > when sending the packet to the stack, as can be seen in
> > the trace below.
> >
> > Fix this by calling postpush/postpull checksum calculation
> > which will update the hw csum if needed.
>
> > -static void set_ipv6_fl(struct ipv6hdr *nh, u32 fl, u32 mask)
> > +static void set_ipv6_dsfield(struct sk_buff *skb, struct ipv6hdr *nh, __u8
> > ipv6_tclass, __u8 mask)
> > {
> > + skb_postpull_rcsum(skb, nh, 4);
> > +
> > + ipv6_change_dsfield(nh, ~mask, ipv6_tclass);
> > +
> > + skb_postpush_rcsum(skb, nh, 4);
> > +}
>
> The calls seem a little heavy for single byte replacements.
> Can you instead add a helper based on csum_replace4() maybe?
>
> BTW doesn't pedit have the same problem?
>
I don't think they are heavier then csum_replace4, but they are more
bulletproof in my opinion, since they handle both the COMPLETE and PARTIAL
csum cases (in __skb_postpull_rcsum()) and resemble what editing of the
packet should have done - pull the header, edit, and then push it back.
RE pedit, not really the issue here, but i guess pedit should be followed
by act_csum with the relevant checksum fields (as we do when offloading
ovs set() action to tc pedit + csum actions).
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
