I would love you to look at skb_copy_and_hash_datagram_iter as these changes will require an ack from you.My first impression is that we now have this kind of code pattern in at least two main places and now this will be a third. I know that nobody likes callbacks because of spectre, but all of these cases could be done with something like: int __skb_datagram_iter(const struct sk_buff *skb, int offset, struct iov_iter *to, int len, int (*cb)(void *, int, struct iov_iter *, void *), void *data) { ... n = cb(skb->data + offset, copy, to, data); ... } You get the idea. Then we have one version of all the loops and the different (copy, copy+csum, copy+hash) cases all can be handled by __skb_datagram_iter() but just with a different 'cb' and private 'data'.
I already thought about that, but the fact that we copy both a buffer and a page to the iter (in the most general case) we'd have to carry two callbacks for indirection.. That wasn't something I thought as acceptable... I guess we could rework skb_copy_datagram_iter to not call copy_page_to_iter and open-code kmapping so we can get away with a single code path? Unless I'm missing something? Also, looking a bit closer there is a slight difference between the copy vs. the copy_and_csum variants. copy allows for a short_copy if we copy less than we expect while the csum faults it. I'm thinking that the copy_and_hash variant should also fault? Although I'm not sure I understand the fault entirely as csum is supposed to be cumulative, any insight?
