All of lore.kernel.org
 help / color / mirror / Atom feed
From: <Markus.Amend@telekom.de>
To: <davem@davemloft.net>
Cc: <linux-kernel@vger.kernel.org>, <netdev@vger.kernel.org>,
	<dccp@vger.kernel.org>
Subject: RE: [PATCH v3] net: dccp: Checksum verification enhancement
Date: Thu, 13 Jun 2019 06:51:34 +0000	[thread overview]
Message-ID: <FRAPR01MB117066D965524B71A7E18FA0FAEF0@FRAPR01MB1170.DEUPRD01.PROD.OUTLOOK.DE> (raw)
In-Reply-To: <20190505.095309.439816991626967361.davem@davemloft.net>

Hi David,

Yes, you are right, I overlooked this. Unfortunately the current receive process in the DCCP layer does from my view not properly support the skb->ip_summed flag verification, because the checksum validation takes place at different places. This would require some dirty hacks...

I see two options.

1. Adding the ip_summed flag verification 


or 2. Learn from the UDP stack

Since UDP/UDP-Lite are very similar to DCCP, at least from a checksum verification point, I ask myself if it would make sense to re-work DCCP's receive process according to the one of UDP/UDP-Lite? 
The relevant process in the udp stack (for IPv4) I identified therefore, can be found in /net/ipv4/udp.c, within the function __udp4_lib_rcv. There it is done, compared to DCCP, the other way round it starts with an udp4_csum_init and most likely a later udp_lib_checksum_complete. Both consider skb->ip_summed. If we would implement similar functions into the DCCP stack and adapt the DCCP rcv checksum validation process to the one in UDP could make probably more sense?!


Personally I prefer the second option, what do you think?

BR

Markus


> -----Original Message-----
> From: David Miller <davem@davemloft.net>
> Sent: Sonntag, 5. Mai 2019 18:53
> To: Amend, Markus <Markus.Amend@telekom.de>
> Cc: linux-kernel@vger.kernel.org; netdev@vger.kernel.org;
> dccp@vger.kernel.org
> Subject: Re: [PATCH v3] net: dccp: Checksum verification enhancement
> 
> From: <Markus.Amend@telekom.de>
> Date: Tue, 30 Apr 2019 16:11:07 +0000
> 
> > The current patch modifies the checksum verification of a received
> > DCCP packet, by adding the function __skb_checksum_validate into the
> > dccp_vX_rcv routine. The purpose of the modification is to allow the
> > verification of the skb->ip_summed flags during the checksum
> > validation process (for checksum offload purposes). As
> > __skb_checksum_validate covers the functionalities of skb_checksum and
> > dccp_vX_csum_finish they are needless and therefore removed.
> >
> > Signed-off-by: Nathalie Romo Moreno <natha.ro.moreno@gmail.com>
> > Signed-off-by: Markus Amend <markus.amend@telekom.de>
> 
> I don't see how this can be correct as you're not taking the csum coverage
> value into consideration at all.

WARNING: multiple messages have this Message-ID (diff)
From: <Markus.Amend@telekom.de>
To: dccp@vger.kernel.org
Subject: RE: [PATCH v3] net: dccp: Checksum verification enhancement
Date: Thu, 13 Jun 2019 06:51:34 +0000	[thread overview]
Message-ID: <FRAPR01MB117066D965524B71A7E18FA0FAEF0@FRAPR01MB1170.DEUPRD01.PROD.OUTLOOK.DE> (raw)

Hi David,

Yes, you are right, I overlooked this. Unfortunately the current receive process in the DCCP layer does from my view not properly support the skb->ip_summed flag verification, because the checksum validation takes place at different places. This would require some dirty hacks...

I see two options.

1. Adding the ip_summed flag verification 


or 2. Learn from the UDP stack

Since UDP/UDP-Lite are very similar to DCCP, at least from a checksum verification point, I ask myself if it would make sense to re-work DCCP's receive process according to the one of UDP/UDP-Lite? 
The relevant process in the udp stack (for IPv4) I identified therefore, can be found in /net/ipv4/udp.c, within the function __udp4_lib_rcv. There it is done, compared to DCCP, the other way round it starts with an udp4_csum_init and most likely a later udp_lib_checksum_complete. Both consider skb->ip_summed. If we would implement similar functions into the DCCP stack and adapt the DCCP rcv checksum validation process to the one in UDP could make probably more sense?!


Personally I prefer the second option, what do you think?

BR

Markus


> -----Original Message-----
> From: David Miller <davem@davemloft.net>
> Sent: Sonntag, 5. Mai 2019 18:53
> To: Amend, Markus <Markus.Amend@telekom.de>
> Cc: linux-kernel@vger.kernel.org; netdev@vger.kernel.org;
> dccp@vger.kernel.org
> Subject: Re: [PATCH v3] net: dccp: Checksum verification enhancement
> 
> From: <Markus.Amend@telekom.de>
> Date: Tue, 30 Apr 2019 16:11:07 +0000
> 
> > The current patch modifies the checksum verification of a received
> > DCCP packet, by adding the function __skb_checksum_validate into the
> > dccp_vX_rcv routine. The purpose of the modification is to allow the
> > verification of the skb->ip_summed flags during the checksum
> > validation process (for checksum offload purposes). As
> > __skb_checksum_validate covers the functionalities of skb_checksum and
> > dccp_vX_csum_finish they are needless and therefore removed.
> >
> > Signed-off-by: Nathalie Romo Moreno <natha.ro.moreno@gmail.com>
> > Signed-off-by: Markus Amend <markus.amend@telekom.de>
> 
> I don't see how this can be correct as you're not taking the csum coverage
> value into consideration at all.

  reply	other threads:[~2019-06-13 16:40 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-30 16:11 [PATCH v3] net: dccp: Checksum verification enhancement Markus.Amend
2019-04-30 16:11 ` Markus.Amend
2019-05-05 16:53 ` David Miller
2019-05-05 16:53   ` David Miller
2019-06-13  6:51   ` Markus.Amend [this message]
2019-06-13  6:51     ` Markus.Amend

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=FRAPR01MB117066D965524B71A7E18FA0FAEF0@FRAPR01MB1170.DEUPRD01.PROD.OUTLOOK.DE \
    --to=markus.amend@telekom.de \
    --cc=davem@davemloft.net \
    --cc=dccp@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.