* [PATCH net-next 3/3] macsec: add brackets and indentation after calling macsec_decrypt
@ 2019-06-30 20:46 Andreas Steinmetz
2019-07-01 2:05 ` Willem de Bruijn
0 siblings, 1 reply; 5+ messages in thread
From: Andreas Steinmetz @ 2019-06-30 20:46 UTC (permalink / raw)
To: netdev; +Cc: Sabrina Dubroca
At this point, skb could only be a valid pointer, so this patch does
not introduce any functional change.
Signed-off-by: Andreas Steinmetz <ast@domdv.de>
--- a/drivers/net/macsec.c 2019-06-30 22:05:17.785683634 +0200
+++ b/drivers/net/macsec.c 2019-06-30 22:05:20.526171178 +0200
@@ -1205,21 +1205,22 @@
/* Disabled && !changed text => skip validation */
if (hdr->tci_an & MACSEC_TCI_C ||
- secy->validate_frames != MACSEC_VALIDATE_DISABLED)
+ secy->validate_frames != MACSEC_VALIDATE_DISABLED) {
skb = macsec_decrypt(skb, dev, rx_sa, sci, secy);
- if (IS_ERR(skb)) {
- /* the decrypt callback needs the reference */
- if (PTR_ERR(skb) != -EINPROGRESS) {
- macsec_rxsa_put(rx_sa);
- macsec_rxsc_put(rx_sc);
+ if (IS_ERR(skb)) {
+ /* the decrypt callback needs the reference */
+ if (PTR_ERR(skb) != -EINPROGRESS) {
+ macsec_rxsa_put(rx_sa);
+ macsec_rxsc_put(rx_sc);
+ }
+ rcu_read_unlock();
+ return RX_HANDLER_CONSUMED;
}
- rcu_read_unlock();
- return RX_HANDLER_CONSUMED;
- }
- if (!macsec_post_decrypt(skb, secy, pn))
- goto drop;
+ if (!macsec_post_decrypt(skb, secy, pn))
+ goto drop;
+ }
deliver:
macsec_finalize_skb(skb, secy->icv_len,
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next 3/3] macsec: add brackets and indentation after calling macsec_decrypt
2019-06-30 20:46 [PATCH net-next 3/3] macsec: add brackets and indentation after calling macsec_decrypt Andreas Steinmetz
@ 2019-07-01 2:05 ` Willem de Bruijn
2019-07-01 13:21 ` Sabrina Dubroca
0 siblings, 1 reply; 5+ messages in thread
From: Willem de Bruijn @ 2019-07-01 2:05 UTC (permalink / raw)
To: Andreas Steinmetz; +Cc: Network Development, Sabrina Dubroca
On Sun, Jun 30, 2019 at 4:48 PM Andreas Steinmetz <ast@domdv.de> wrote:
>
> At this point, skb could only be a valid pointer, so this patch does
> not introduce any functional change.
Previously, macsec_post_decrypt could be called on the original skb if
the initial condition was false and macsec_decrypt is skipped. That
was probably unintended. Either way, then this is a functional change,
and perhaps a bugfix?
> Signed-off-by: Andreas Steinmetz <ast@domdv.de>
>
> --- a/drivers/net/macsec.c 2019-06-30 22:05:17.785683634 +0200
> +++ b/drivers/net/macsec.c 2019-06-30 22:05:20.526171178 +0200
> @@ -1205,21 +1205,22 @@
>
> /* Disabled && !changed text => skip validation */
> if (hdr->tci_an & MACSEC_TCI_C ||
> - secy->validate_frames != MACSEC_VALIDATE_DISABLED)
> + secy->validate_frames != MACSEC_VALIDATE_DISABLED) {
> skb = macsec_decrypt(skb, dev, rx_sa, sci, secy);
>
> - if (IS_ERR(skb)) {
> - /* the decrypt callback needs the reference */
> - if (PTR_ERR(skb) != -EINPROGRESS) {
> - macsec_rxsa_put(rx_sa);
> - macsec_rxsc_put(rx_sc);
> + if (IS_ERR(skb)) {
> + /* the decrypt callback needs the reference */
> + if (PTR_ERR(skb) != -EINPROGRESS) {
> + macsec_rxsa_put(rx_sa);
> + macsec_rxsc_put(rx_sc);
> + }
> + rcu_read_unlock();
> + return RX_HANDLER_CONSUMED;
> }
> - rcu_read_unlock();
> - return RX_HANDLER_CONSUMED;
> - }
>
> - if (!macsec_post_decrypt(skb, secy, pn))
> - goto drop;
> + if (!macsec_post_decrypt(skb, secy, pn))
> + goto drop;
> + }
>
> deliver:
> macsec_finalize_skb(skb, secy->icv_len,
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next 3/3] macsec: add brackets and indentation after calling macsec_decrypt
2019-07-01 2:05 ` Willem de Bruijn
@ 2019-07-01 13:21 ` Sabrina Dubroca
2019-07-02 4:37 ` Andreas Steinmetz
0 siblings, 1 reply; 5+ messages in thread
From: Sabrina Dubroca @ 2019-07-01 13:21 UTC (permalink / raw)
To: Willem de Bruijn; +Cc: Andreas Steinmetz, Network Development
2019-06-30, 22:05:41 -0400, Willem de Bruijn wrote:
> On Sun, Jun 30, 2019 at 4:48 PM Andreas Steinmetz <ast@domdv.de> wrote:
> >
> > At this point, skb could only be a valid pointer, so this patch does
> > not introduce any functional change.
>
> Previously, macsec_post_decrypt could be called on the original skb if
> the initial condition was false and macsec_decrypt is skipped. That
> was probably unintended. Either way, then this is a functional change,
> and perhaps a bugfix?
Ouch, I missed that when Andreas sent me that patch before. No, it is
actually intended. If we skip macsec_decrypt(), we should still
account for that packet in the InPktsUnchecked/InPktsDelayed
counters. That's in Figure 10-5 in the standard.
Thanks for catching this, Willem. That patch should only move the
IS_ERR(skb) case under the block where macsec_decrypt() is called, but
not move the call to macsec_post_decrypt().
> > Signed-off-by: Andreas Steinmetz <ast@domdv.de>
> >
> > --- a/drivers/net/macsec.c 2019-06-30 22:05:17.785683634 +0200
> > +++ b/drivers/net/macsec.c 2019-06-30 22:05:20.526171178 +0200
> > @@ -1205,21 +1205,22 @@
> >
> > /* Disabled && !changed text => skip validation */
> > if (hdr->tci_an & MACSEC_TCI_C ||
> > - secy->validate_frames != MACSEC_VALIDATE_DISABLED)
> > + secy->validate_frames != MACSEC_VALIDATE_DISABLED) {
> > skb = macsec_decrypt(skb, dev, rx_sa, sci, secy);
> >
> > - if (IS_ERR(skb)) {
> > - /* the decrypt callback needs the reference */
> > - if (PTR_ERR(skb) != -EINPROGRESS) {
> > - macsec_rxsa_put(rx_sa);
> > - macsec_rxsc_put(rx_sc);
> > + if (IS_ERR(skb)) {
> > + /* the decrypt callback needs the reference */
> > + if (PTR_ERR(skb) != -EINPROGRESS) {
> > + macsec_rxsa_put(rx_sa);
> > + macsec_rxsc_put(rx_sc);
> > + }
> > + rcu_read_unlock();
> > + return RX_HANDLER_CONSUMED;
> > }
> > - rcu_read_unlock();
> > - return RX_HANDLER_CONSUMED;
> > - }
> >
> > - if (!macsec_post_decrypt(skb, secy, pn))
> > - goto drop;
> > + if (!macsec_post_decrypt(skb, secy, pn))
> > + goto drop;
> > + }
> >
> > deliver:
> > macsec_finalize_skb(skb, secy->icv_len,
> >
--
Sabrina
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next 3/3] macsec: add brackets and indentation after calling macsec_decrypt
2019-07-01 13:21 ` Sabrina Dubroca
@ 2019-07-02 4:37 ` Andreas Steinmetz
2019-07-02 14:35 ` Willem de Bruijn
0 siblings, 1 reply; 5+ messages in thread
From: Andreas Steinmetz @ 2019-07-02 4:37 UTC (permalink / raw)
To: Sabrina Dubroca, Willem de Bruijn; +Cc: Network Development
Ouch, I missed that when Andreas sent me that patch before. No, it is
> actually intended. If we skip macsec_decrypt(), we should still
> account for that packet in the InPktsUnchecked/InPktsDelayed
> counters. That's in Figure 10-5 in the standard.
>
> Thanks for catching this, Willem. That patch should only move the
> IS_ERR(skb) case under the block where macsec_decrypt() is called, but
> not move the call to macsec_post_decrypt().
Updated patch below.
Signed-off-by: Andreas Steinmetz <ast@domdv.de>
--- a/drivers/net/macsec.c 2019-07-02 06:31:27.550120145 +0200
+++ b/drivers/net/macsec.c 2019-07-02 06:33:38.637599529 +0200
@@ -1205,17 +1205,18 @@
/* Disabled && !changed text => skip validation */
if (hdr->tci_an & MACSEC_TCI_C ||
- secy->validate_frames != MACSEC_VALIDATE_DISABLED)
+ secy->validate_frames != MACSEC_VALIDATE_DISABLED) {
skb = macsec_decrypt(skb, dev, rx_sa, sci, secy);
- if (IS_ERR(skb)) {
- /* the decrypt callback needs the reference */
- if (PTR_ERR(skb) != -EINPROGRESS) {
- macsec_rxsa_put(rx_sa);
- macsec_rxsc_put(rx_sc);
+ if (IS_ERR(skb)) {
+ /* the decrypt callback needs the reference */
+ if (PTR_ERR(skb) != -EINPROGRESS) {
+ macsec_rxsa_put(rx_sa);
+ macsec_rxsc_put(rx_sc);
+ }
+ rcu_read_unlock();
+ return RX_HANDLER_CONSUMED;
}
- rcu_read_unlock();
- return RX_HANDLER_CONSUMED;
}
if (!macsec_post_decrypt(skb, secy, pn))
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next 3/3] macsec: add brackets and indentation after calling macsec_decrypt
2019-07-02 4:37 ` Andreas Steinmetz
@ 2019-07-02 14:35 ` Willem de Bruijn
0 siblings, 0 replies; 5+ messages in thread
From: Willem de Bruijn @ 2019-07-02 14:35 UTC (permalink / raw)
To: Andreas Steinmetz; +Cc: Sabrina Dubroca, Network Development
On Tue, Jul 2, 2019 at 12:38 AM Andreas Steinmetz <ast@domdv.de> wrote:
>
> Ouch, I missed that when Andreas sent me that patch before. No, it is
> > actually intended. If we skip macsec_decrypt(), we should still
> > account for that packet in the InPktsUnchecked/InPktsDelayed
> > counters. That's in Figure 10-5 in the standard.
> >
> > Thanks for catching this, Willem. That patch should only move the
> > IS_ERR(skb) case under the block where macsec_decrypt() is called, but
> > not move the call to macsec_post_decrypt().
>
> Updated patch below.
>
> Signed-off-by: Andreas Steinmetz <ast@domdv.de>
When making a change in a patch set, please resubmit the entire
patchset (with a v2, and record the changelog).
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-07-02 14:36 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-30 20:46 [PATCH net-next 3/3] macsec: add brackets and indentation after calling macsec_decrypt Andreas Steinmetz
2019-07-01 2:05 ` Willem de Bruijn
2019-07-01 13:21 ` Sabrina Dubroca
2019-07-02 4:37 ` Andreas Steinmetz
2019-07-02 14:35 ` Willem de Bruijn
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.