Re: bge(4): enable TCP/UDP checksum offload

2012-11-03 Thread Christian Weisgerber
Brad Smith:

> Here is another revision but disabling the UDP checksum offload.
> There is a bug that results in some UDP packets having a 0 checksum.

Oh, right.  I assume you are referencing this FreeBSD commit?
http://svnweb.freebsd.org/base/head/sys/dev/bge/if_bge.c?r1=211595&r2=211596&diff_format=u

In that case I suggest we also copy the relevant portion of the comment:

+* It seems all Broadcom controllers have a bug that can generate UDP
+* datagrams with checksum value 0 when TX UDP checksum offloading is
+* enabled.  Generating UDP checksum value 0 is RFC 768 violation.

-- 
Christian "naddy" Weisgerber  na...@mips.inka.de



Re: bge(4): enable TCP/UDP checksum offload

2012-11-03 Thread Brad Smith
On Sat, Nov 03, 2012 at 09:41:08PM +, Christian Weisgerber wrote:
> Previously, we couldn't enable TCP/UDP transmit checksum offload
> on chipsets that require the checksum field to be initialized with
> the pseudo-header checksum because this would break rdr-to 127.0.0.1.
> 
> Henning's epic rewrite of the checksum handling has fixed this
> problem, so let's enable TCP/UDP checksumming on bge(4).  This
> _might_ break on some of the gazillion chipset versions.  There's
> no way to find out but try.
> 
> FWIW, it works fine with VLAN on this one:
> bge0 at pci0 dev 5 function 0 "Broadcom BCM5702X" rev 0x02, BCM5702/5703
> A2 (0x1002)

Here is another revision but disabling the UDP checksum offload.
There is a bug that results in some UDP packets having a 0 checksum.


Index: if_bge.c
===
RCS file: /home/cvs/src/sys/dev/pci/if_bge.c,v
retrieving revision 1.312
diff -u -p -r1.312 if_bge.c
--- if_bge.c13 Sep 2012 04:15:18 -  1.312
+++ if_bge.c2 Nov 2012 04:15:40 -
@@ -2157,10 +2157,7 @@ bge_attach(struct device *parent, struct
 * to hardware bugs.
 */
if (sc->bge_chipid != BGE_CHIPID_BCM5700_B0)
-   ifp->if_capabilities |= IFCAP_CSUM_IPv4;
-#if 0  /* TCP/UDP checksum offload breaks with pf(4) */
-   ifp->if_capabilities |= IFCAP_CSUM_TCPv4|IFCAP_CSUM_UDPv4;
-#endif
+   ifp->if_capabilities |= IFCAP_CSUM_IPv4|IFCAP_CSUM_TCPv4;
 
if (BGE_IS_JUMBO_CAPABLE(sc))
ifp->if_hardmtu = BGE_JUMBO_MTU;

-- 
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.



Re: bge(4): enable TCP/UDP checksum offload

2012-11-03 Thread Christiano F. Haesbaert
On 03/11/2012, Christian Weisgerber  wrote:
> Previously, we couldn't enable TCP/UDP transmit checksum offload
> on chipsets that require the checksum field to be initialized with
> the pseudo-header checksum because this would break rdr-to 127.0.0.1.
>
> Henning's epic rewrite of the checksum handling has fixed this
> problem, so let's enable TCP/UDP checksumming on bge(4).  This
> _might_ break on some of the gazillion chipset versions.  There's
> no way to find out but try.
>
> FWIW, it works fine with VLAN on this one:
> bge0 at pci0 dev 5 function 0 "Broadcom BCM5702X" rev 0x02, BCM5702/5703
> A2 (0x1002)
>

Surprisingly I did the same thing at my tree today, I have a:

bge0 at pci3 dev 0 function 0 "Broadcom BCM5723" rev 0x10, BCM5784 A1
(0x5784100)

I run x61s with the hwcsum enabled on em, and can confirm that
henning's commit fixed pf-nat.



em(4): enable TCP/UDP checksum offload

2012-11-03 Thread Christian Weisgerber
Like bge(4), we previously couldn't enable TCP/UDP transmit checksum
offload on em(4).  We can now.

Works fine here on
  em0 at pci1 dev 1 function 0 "Intel PRO/1000MT (82540EM)" rev 0x02
and with VLAN on
  em0 at pci5 dev 0 function 0 "Intel PRO/1000 MT (82574L)" rev 0x00

Does anybody see any value in keeping the #ifdef around?

Index: dev/pci/if_em.c
===
RCS file: /cvs/src/sys/dev/pci/if_em.c,v
retrieving revision 1.267
diff -u -p -r1.267 if_em.c
--- dev/pci/if_em.c 16 Aug 2012 09:31:53 -  1.267
+++ dev/pci/if_em.c 3 Nov 2012 01:53:32 -
@@ -211,10 +211,8 @@ int  em_rxfill(struct em_softc *);
 void em_rxeof(struct em_softc *);
 void em_receive_checksum(struct em_softc *, struct em_rx_desc *,
 struct mbuf *);
-#ifdef EM_CSUM_OFFLOAD
 void em_transmit_checksum_setup(struct em_softc *, struct mbuf *,
u_int32_t *, u_int32_t *);
-#endif
 void em_iff(struct em_softc *);
 #ifdef EM_DEBUG
 void em_print_hw_stats(struct em_softc *);
@@ -1121,14 +1119,10 @@ em_encap(struct em_softc *sc, struct mbu
if (map->dm_nsegs > sc->num_tx_desc_avail - 2)
goto fail;
 
-#ifdef EM_CSUM_OFFLOAD
if (sc->hw.mac_type >= em_82543)
em_transmit_checksum_setup(sc, m_head, &txd_upper, &txd_lower);
else
txd_upper = txd_lower = 0;
-#else
-   txd_upper = txd_lower = 0;
-#endif
 
i = sc->next_avail_tx_desc;
if (sc->pcix_82544)
@@ -1853,10 +1847,8 @@ em_setup_interface(struct em_softc *sc)
ifp->if_capabilities |= IFCAP_VLAN_HWTAGGING;
 #endif
 
-#ifdef EM_CSUM_OFFLOAD
if (sc->hw.mac_type >= em_82543)
ifp->if_capabilities |= IFCAP_CSUM_TCPv4|IFCAP_CSUM_UDPv4;
-#endif
 
/* 
 * Specify the media types supported by this adapter and register
@@ -2275,7 +2267,6 @@ em_free_transmit_structures(struct em_so
sc->txtag = NULL;
 }
 
-#ifdef EM_CSUM_OFFLOAD
 /*
  *
  *  The offload context needs to be set when we transfer the first
@@ -2356,7 +2347,6 @@ em_transmit_checksum_setup(struct em_sof
sc->num_tx_desc_avail--;
sc->next_avail_tx_desc = curr_txd;
 }
-#endif /* EM_CSUM_OFFLOAD */
 
 /**
  *
-- 
Christian "naddy" Weisgerber  na...@mips.inka.de



bge(4): enable TCP/UDP checksum offload

2012-11-03 Thread Christian Weisgerber
Previously, we couldn't enable TCP/UDP transmit checksum offload
on chipsets that require the checksum field to be initialized with
the pseudo-header checksum because this would break rdr-to 127.0.0.1.

Henning's epic rewrite of the checksum handling has fixed this
problem, so let's enable TCP/UDP checksumming on bge(4).  This
_might_ break on some of the gazillion chipset versions.  There's
no way to find out but try.

FWIW, it works fine with VLAN on this one:
bge0 at pci0 dev 5 function 0 "Broadcom BCM5702X" rev 0x02, BCM5702/5703
A2 (0x1002)

Index: if_bge.c
===
RCS file: /cvs/src/sys/dev/pci/if_bge.c,v
retrieving revision 1.312
diff -u -p -r1.312 if_bge.c
--- if_bge.c13 Sep 2012 04:15:18 -  1.312
+++ if_bge.c2 Nov 2012 22:13:39 -
@@ -2157,10 +2157,8 @@ bge_attach(struct device *parent, struct
 * to hardware bugs.
 */
if (sc->bge_chipid != BGE_CHIPID_BCM5700_B0)
-   ifp->if_capabilities |= IFCAP_CSUM_IPv4;
-#if 0  /* TCP/UDP checksum offload breaks with pf(4) */
-   ifp->if_capabilities |= IFCAP_CSUM_TCPv4|IFCAP_CSUM_UDPv4;
-#endif
+   ifp->if_capabilities |= IFCAP_CSUM_IPv4 | IFCAP_CSUM_TCPv4 |
+   IFCAP_CSUM_UDPv4;
 
if (BGE_IS_JUMBO_CAPABLE(sc))
ifp->if_hardmtu = BGE_JUMBO_MTU;
-- 
Christian "naddy" Weisgerber  na...@mips.inka.de



pflogd(8): two if_exists() fixes

2012-11-03 Thread Lawrence Teo
Quick background: In pflogd(8), the if_exists() function tests if a
given pflogX interface exists.  It returns 1 (if it exists) or 0 (if
not).

This diff fixes two issues with if_exists():

1. if_exists() opens a socket to test the pflogX interface exists.  If
   the interface does not exist, the function will return without
   closing that socket.

2. There is an incorrect call to if_exists() in the code:

if (!if_exists(interface) == -1) {

Comments/OK?

Lawrence


Index: pflogd.c
===
RCS file: /cvs/src/sbin/pflogd/pflogd.c,v
retrieving revision 1.48
diff -U4 -p -r1.48 pflogd.c
--- pflogd.c5 Mar 2012 11:50:16 -   1.48
+++ pflogd.c3 Nov 2012 18:31:43 -
@@ -202,9 +202,9 @@ set_pcap_filter(void)
 
 int
 if_exists(char *ifname)
 {
-   int s;
+   int s, ret = 1;
struct ifreq ifr;
struct if_data ifrdat;
 
if ((s = socket(AF_INET, SOCK_DGRAM, 0)) == -1)
@@ -214,13 +214,13 @@ if_exists(char *ifname)
sizeof(ifr.ifr_name))
errx(1, "main ifr_name: strlcpy");
ifr.ifr_data = (caddr_t)&ifrdat;
if (ioctl(s, SIOCGIFDATA, (caddr_t)&ifr) == -1)
-   return (0);
+   ret = 0;
if (close(s))
err(1, "close");
 
-   return (1);
+   return (ret);
 }
 
 int
 init_pcap(void)
@@ -689,9 +689,9 @@ main(int argc, char **argv)
while (1) {
np = pcap_dispatch(hpcap, PCAP_NUM_PKTS,
phandler, (u_char *)dpcap);
if (np < 0) {
-   if (!if_exists(interface) == -1) {
+   if (!if_exists(interface)) {
logmsg(LOG_NOTICE, "interface %s went away",
interface);
ret = -1;
break;



mg: remove b_undopos

2012-11-03 Thread Florian Obser
Found while investigating a llvm report. Since this doesn't actually
do anything, the dot handling of undo (i.e. move dot to where an undo
is happening) is fine afaic, I'd like to remove this. Unless someone
has a diff to make this useful...

Also note the rather elaborate way an int is set to zero ;)

diff --git buffer.c buffer.c
index a2e84ab..a01f92d 100644
--- buffer.c
+++ buffer.c
@@ -543,7 +543,6 @@ bnew(const char *bname)
bp->b_nmodes = defb_nmodes;
TAILQ_INIT(&bp->b_undo);
bp->b_undoptr = NULL;
-   memset(&bp->b_undopos, 0, sizeof(bp->b_undopos));
i = 0;
do {
bp->b_modes[i] = defb_modes[i];
diff --git def.h def.h
index 2efe5cc..39c6409 100644
--- def.h
+++ def.h
@@ -260,7 +260,6 @@ struct buffer {
char b_cwd[NFILEN]; /* working directory */
struct fileinfo  b_fi;  /* File attributes   */
struct undoq b_undo;/* Undo actions list */
-   int  b_undopos; /* Where we were during last undo */
struct undo_rec *b_undoptr;
int  b_dotline; /* Line number of dot */
int  b_markline;/* Line number of mark */
diff --git undo.c undo.c
index 1efd2a1..2140b66 100644
--- undo.c
+++ undo.c
@@ -464,15 +464,13 @@ undo(int f, int n)
struct undo_rec *ptr, *nptr;
int  done, rval;
struct line *lp;
-   int  offset, save, dot;
+   int  offset, save;
static int   nulled = FALSE;
int  lineno;
 
if (n < 0)
return (FALSE);
 
-   dot = find_dot(curwp->w_dotp, curwp->w_doto);
-
ptr = curbp->b_undoptr;
 
/* first invocation, make ptr point back to the top of the list */
@@ -573,13 +571,8 @@ undo(int f, int n)
 
ewprintf("Undo!");
}
-   /*
-* Record where we are. (we have to save our new position at the end
-* since we change the dot when undoing)
-*/
-   curbp->b_undoptr = ptr;
 
-   curbp->b_undopos = find_dot(curwp->w_dotp, curwp->w_doto);
+   curbp->b_undoptr = ptr;
 
return (rval);
 }

-- 
I'm not entirely sure you are real.



Re: Goodbye to you my file descriptor

2012-11-03 Thread Loganaden Velvindron
Thanks for fixing my mistake :-)


On Sat, Nov 3, 2012 at 6:57 PM, Christiano F. Haesbaert
 wrote:
> On Tue, Oct 30, 2012 at 04:44:36PM +0100, rustyBSD wrote:
>> Le 30/10/2012 15:32, Christiano F. Haesbaert a ?crit :
>> > That should be an access(2) call.
>> Yes.Something like this - also moved len to size_t,
>> as strlen() is size_t:
>>
>>
>> --- dired.cWed Mar 14 14:56:35 2012
>> +++ dired.cTue Oct 30 16:23:00 2012
>> @@ -724,9 +724,10 @@
>>  dired_(char *dname)
>>  {
>>  struct buffer*bp;
>> -int len, i;
>> +int i;
>> +size_t len;
>>
>> -if ((fopen(dname,"r")) == NULL) {
>> +if (access(dname, R_OK) == -1) {
>>  if (errno == EACCES)
>>  ewprintf("Permission denied");
>>  return (NULL);
>>
>
> Your diff got mangled, it replaced tabs for spaces, anyway, I think we
> should also check for X_OK, otherwise we can't list the directory
> anyway.
>
> We still get the message "File is not readable", but I believe it's more
> useful than showing an empty directory.
>
> Index: dired.c
> ===
> RCS file: /cvs/src/usr.bin/mg/dired.c,v
> retrieving revision 1.51
> diff -d -u -p -r1.51 dired.c
> --- dired.c 14 Mar 2012 13:56:35 -  1.51
> +++ dired.c 3 Nov 2012 14:47:18 -
> @@ -724,9 +724,10 @@ struct buffer *
>  dired_(char *dname)
>  {
> struct buffer   *bp;
> -   int  len, i;
> +   int  i;
> +   size_t   len;
>
> -   if ((fopen(dname,"r")) == NULL) {
> +   if ((access(dname, R_OK | X_OK)) == -1) {
> if (errno == EACCES)
> ewprintf("Permission denied");
> return (NULL);
>



-- 
Brightest day,
Blackest night,
No bug shall escape my sight,
And those who worship evil's mind,
be wary of my powers,
puffy lantern's light !



Re: Goodbye to you my file descriptor

2012-11-03 Thread Christiano F. Haesbaert
On Tue, Oct 30, 2012 at 04:44:36PM +0100, rustyBSD wrote:
> Le 30/10/2012 15:32, Christiano F. Haesbaert a ?crit :
> > That should be an access(2) call.
> Yes.Something like this - also moved len to size_t,
> as strlen() is size_t:
> 
> 
> --- dired.cWed Mar 14 14:56:35 2012
> +++ dired.cTue Oct 30 16:23:00 2012
> @@ -724,9 +724,10 @@
>  dired_(char *dname)
>  {
>  struct buffer*bp;
> -int len, i;
> +int i;
> +size_t len;
>  
> -if ((fopen(dname,"r")) == NULL) {
> +if (access(dname, R_OK) == -1) {
>  if (errno == EACCES)
>  ewprintf("Permission denied");
>  return (NULL);
> 

Your diff got mangled, it replaced tabs for spaces, anyway, I think we
should also check for X_OK, otherwise we can't list the directory
anyway. 

We still get the message "File is not readable", but I believe it's more
useful than showing an empty directory.

Index: dired.c
===
RCS file: /cvs/src/usr.bin/mg/dired.c,v
retrieving revision 1.51
diff -d -u -p -r1.51 dired.c
--- dired.c 14 Mar 2012 13:56:35 -  1.51
+++ dired.c 3 Nov 2012 14:47:18 -
@@ -724,9 +724,10 @@ struct buffer *
 dired_(char *dname)
 {
struct buffer   *bp;
-   int  len, i;
+   int  i;
+   size_t   len;
 
-   if ((fopen(dname,"r")) == NULL) {
+   if ((access(dname, R_OK | X_OK)) == -1) {
if (errno == EACCES)
ewprintf("Permission denied");
return (NULL);



softraid_crypto.c and bioctl.c: select encryption algorithm and parameters

2012-11-03 Thread Thiebaud Weksteen
Hi all,

Here is a patch I've made to select the algorithm and its
parameters used for a softraid encryption. I added an extra field
to the kdfinfo structure to retrieve the algorithm type from
userland (This structure may need to be renamed in the future).
I also modified bioctl to allow this extra option. (And updated
the man page as well).

Finally, I added the ioctl discipline SR_IOCTL_GET_ALGORITHM to
retrieve the encryption algorithm name from userland. Now, bioctl -i
returns something like:
# bioctl -i sd2
Volume  Status   Size Device
softraid0 2 Online   16180224 sd2 CRYPTO AES_XTS_256
 0 Online   16180224 2:0.0   noencl 

Comments are welcomed.

Thiebaud Weksteen

Index: softraid_crypto.c
===
RCS file: /cvs/src/sys/dev/softraid_crypto.c,v
retrieving revision 1.82
diff -u -p -u -r1.82 softraid_crypto.c
--- softraid_crypto.c   9 Oct 2012 11:57:33 -   1.82
+++ softraid_crypto.c   27 Oct 2012 09:38:39 -
@@ -151,6 +151,7 @@ sr_crypto_create(struct sr_discipline *s
omi->omi_som->som_length = sizeof(struct sr_meta_crypto);
SLIST_INSERT_HEAD(&sd->sd_meta_opt, omi, omi_link);
sd->mds.mdd_crypto.scr_meta = (struct sr_meta_crypto *)omi->omi_som;
+   sd->mds.mdd_crypto.scr_meta->scm_flags = 0;
sd->sd_meta->ssdi.ssd_opt_no++;

sd->mds.mdd_crypto.key_disk = NULL;
@@ -397,6 +398,20 @@ sr_crypto_get_kdf(struct bioc_createraid
bcopy(&kdfinfo->maskkey, sd->mds.mdd_crypto.scr_maskkey,
sizeof(kdfinfo->maskkey));
}
+   
+   /* copy the requested algorithm, if applicable */
+   if ((kdfinfo->flags & SR_CRYPTOKDF_ALG) &&
+!sd->mds.mdd_crypto.scr_meta->scm_alg) {
+   switch (kdfinfo->alg) {
+   case SR_CRYPTOA_AES_XTS_128:
+   case SR_CRYPTOA_AES_XTS_256:
+   sd->mds.mdd_crypto.scr_meta->scm_alg = kdfinfo->alg;
+   sd->mds.mdd_crypto.scr_meta->scm_flags |= 
SR_CRYPTOF_ALG;
+   break;
+   default:
+   goto out;
+   }
+   }

bc->bc_opaque_status = BIOC_SOINOUT_OK;
rv = 0;
@@ -541,9 +556,6 @@ sr_crypto_create_keys(struct sr_discipli
if (AES_MAXKEYBYTES < sizeof(sd->mds.mdd_crypto.scr_maskkey))
return (1);

-   /* XXX allow user to specify */
-   sd->mds.mdd_crypto.scr_meta->scm_alg = SR_CRYPTOA_AES_XTS_256;
-
/* generate crypto keys */
arc4random_buf(sd->mds.mdd_crypto.scr_key,
sizeof(sd->mds.mdd_crypto.scr_key));
@@ -571,7 +583,7 @@ sr_crypto_create_keys(struct sr_discipli
sr_crypto_dumpkeys(sd);
 #endif

-   sd->mds.mdd_crypto.scr_meta->scm_flags = SR_CRYPTOF_KEY |
+   sd->mds.mdd_crypto.scr_meta->scm_flags |= SR_CRYPTOF_KEY |
SR_CRYPTOF_KDFHINT;

return (0);
@@ -1103,6 +1115,26 @@ sr_crypto_ioctl(struct sr_discipline *sd
DEVNAME(sd->sd_sc), bd->bd_cmd);

switch (bd->bd_cmd) {
+   case SR_IOCTL_GET_ALGORITHM:
+   if(bd->bd_data == NULL)
+   goto bad;
+   switch(sd->mds.mdd_crypto.scr_meta->scm_alg){
+   case SR_CRYPTOA_AES_XTS_128:
+   copyout(SR_CRYPTOA_AES_XTS_128_S, bd->bd_data,
+   sizeof(SR_CRYPTOA_AES_XTS_128_S));
+   break;
+   case SR_CRYPTOA_AES_XTS_256:
+   copyout(SR_CRYPTOA_AES_XTS_256_S, bd->bd_data,
+   sizeof(SR_CRYPTOA_AES_XTS_256_S));
+   break;
+   default:
+   break;
+   }
+
+   rv = 0;
+
+   break;
+   
case SR_IOCTL_GET_KDFHINT:

/* Get KDF hint for userland. */
Index: softraidvar.h
===
RCS file: /cvs/src/sys/dev/softraidvar.h,v
retrieving revision 1.119
diff -u -p -u -r1.119 softraidvar.h
--- softraidvar.h   9 Oct 2012 13:55:36 -   1.119
+++ softraidvar.h   27 Oct 2012 09:38:40 -
@@ -149,11 +149,14 @@ struct sr_meta_crypto {
struct sr_meta_opt_hdr  scm_hdr;
u_int32_t   scm_alg;/* vol crypto algorithm */
 #define SR_CRYPTOA_AES_XTS_128 1
+#define SR_CRYPTOA_AES_XTS_128_S  "AES_XTS_128"
 #define SR_CRYPTOA_AES_XTS_256 2
+#define SR_CRYPTOA_AES_XTS_256_S  "AES_XTS_256"
u_int32_t   scm_flags;  /* key & kdfhint valid */
 #define SR_CRYPTOF_INVALID (0)
 #define SR_CRYPTOF_KEY (1<<0)
 #define SR_CRYPTOF_KDFHINT (1<<1)
+#define SR_CRYPTOF_ALG (1<<2)
u_int32_t   scm_mask_alg;   /* disk key masking crypt alg */
 #define SR_CRYPTOM_AES_ECB_256 1